diff options
author | kewang@google.com <kewang@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-01 02:31:22 +0000 |
---|---|---|
committer | kewang@google.com <kewang@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-01 02:31:22 +0000 |
commit | 0ff0d313c6efc3c42f0f71abec3ad81313598bbe (patch) | |
tree | f7cb4e95cd54c8540c7d84baf188d3b87ede5332 /chrome/browser/safe_browsing | |
parent | e69075f26218e8da4bff9b3b8ab2b355227a2328 (diff) | |
download | chromium_src-0ff0d313c6efc3c42f0f71abec3ad81313598bbe.zip chromium_src-0ff0d313c6efc3c42f0f71abec3ad81313598bbe.tar.gz chromium_src-0ff0d313c6efc3c42f0f71abec3ad81313598bbe.tar.bz2 |
Add the new protocol buffer and malware IP info collection to the client side detection framework.
BUG=176647
Review URL: https://chromiumcodereview.appspot.com/11439004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@185415 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/safe_browsing')
9 files changed, 465 insertions, 28 deletions
diff --git a/chrome/browser/safe_browsing/browser_feature_extractor.cc b/chrome/browser/safe_browsing/browser_feature_extractor.cc index c00d714..bc89109 100644 --- a/chrome/browser/safe_browsing/browser_feature_extractor.cc +++ b/chrome/browser/safe_browsing/browser_feature_extractor.cc @@ -50,6 +50,22 @@ static void AddFeature(const std::string& feature_name, VLOG(2) << "Browser feature: " << feature->name() << " " << feature->value(); } +static void AddMalwareFeature(const std::string& feature_name, + const std::set<std::string>& meta_infos, + double feature_value, + ClientMalwareRequest* request) { + DCHECK(request); + ClientMalwareRequest::Feature* feature = + request->add_feature_map(); + feature->set_name(feature_name); + feature->set_value(feature_value); + for (std::set<std::string>::const_iterator it = meta_infos.begin(); + it != meta_infos.end(); ++it) { + feature->add_metainfo(*it); + } + VLOG(2) << "Browser feature: " << feature->name() << " " << feature->value(); +} + static void AddNavigationFeatures( const std::string& feature_prefix, const NavigationController& controller, @@ -211,14 +227,33 @@ void BrowserFeatureExtractor::ExtractFeatures(const BrowseInfo* info, weak_factory_.GetWeakPtr(), request, callback)); } +void BrowserFeatureExtractor::ExtractMalwareFeatures( + const BrowseInfo* info, + ClientMalwareRequest* request) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + DCHECK(request); + DCHECK(info); + DCHECK_EQ(0U, request->url().find("http:")); + // get the IPs and hosts that match the malware blacklisted IP list. + if (service_) { + for (IPHostMap::const_iterator it = info->ips.begin(); + it != info->ips.end(); ++it) { + if (service_->IsBadIpAddress(it->first)) { + AddMalwareFeature(features::kBadIpFetch + it->first, + it->second, 1.0, request); + } + } + } +} + void BrowserFeatureExtractor::ExtractBrowseInfoFeatures( const BrowseInfo& info, ClientPhishingRequest* request) { if (service_) { - for (std::set<std::string>::const_iterator it = info.ips.begin(); + for (IPHostMap::const_iterator it = info.ips.begin(); it != info.ips.end(); ++it) { - if (service_->IsBadIpAddress(*it)) { - AddFeature(features::kBadIpFetch + *it, 1.0, request); + if (service_->IsBadIpAddress(it->first)) { + AddFeature(features::kBadIpFetch + it->first, 1.0, request); } } } diff --git a/chrome/browser/safe_browsing/browser_feature_extractor.h b/chrome/browser/safe_browsing/browser_feature_extractor.h index 717e150..9a4ba3b 100644 --- a/chrome/browser/safe_browsing/browser_feature_extractor.h +++ b/chrome/browser/safe_browsing/browser_feature_extractor.h @@ -18,6 +18,7 @@ #include "base/basictypes.h" #include "base/callback.h" +#include "base/hash_tables.h" #include "base/memory/scoped_ptr.h" #include "base/sequenced_task_runner_helpers.h" #include "base/time.h" @@ -34,13 +35,16 @@ class WebContents; } namespace safe_browsing { +class ClientMalwareRequest; class ClientPhishingRequest; class ClientSideDetectionService; +typedef std::map<std::string, std::set<std::string> > IPHostMap; + struct BrowseInfo { // List of IPv4 and IPv6 addresses from which content was requested - // while browsing to the |url|. - std::set<std::string> ips; + // together with the hosts on it, while browsing to the |url|. + IPHostMap ips; // If a SafeBrowsing interstitial was shown for the current URL // this will contain the UnsafeResource struct for that URL. @@ -68,6 +72,7 @@ class BrowserFeatureExtractor { // phishing request which was modified by the feature extractor. The // DoneCallback takes ownership of the request object. typedef base::Callback<void(bool, ClientPhishingRequest*)> DoneCallback; + typedef base::Callback<void(bool, ClientMalwareRequest*)> MalwareDoneCallback; // The caller keeps ownership of the tab and service objects and is // responsible for ensuring that they stay valid for the entire @@ -88,6 +93,11 @@ class BrowserFeatureExtractor { ClientPhishingRequest* request, const DoneCallback& callback); + // Extract the malware related features. The request object is owned by the + // caller. + virtual void ExtractMalwareFeatures(const BrowseInfo* info, + ClientMalwareRequest* request); + private: friend class base::DeleteHelper<BrowserFeatureExtractor>; typedef std::pair<ClientPhishingRequest*, DoneCallback> ExtractionData; diff --git a/chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc b/chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc index c31887e..fa538d5 100644 --- a/chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc +++ b/chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc @@ -141,6 +141,26 @@ class BrowserFeatureExtractorTest : public ChromeRenderViewHostTestHarness { } } + void ExtractMalwareFeatures(ClientMalwareRequest* request) { + extractor_->ExtractMalwareFeatures( + browse_info_.get(), request); + } + + void GetMalwareFeatureMap( + const ClientMalwareRequest& request, + std::map<std::string, std::set<std::string> >* features) { + for (int i = 0; i < request.feature_map_size(); ++i) { + const ClientMalwareRequest::Feature& feature = + request.feature_map(i); + EXPECT_EQ(0U, features->count(feature.name())); + std::set<std::string> meta_infos; + for (int j = 0; j < feature.metainfo_size(); ++j) { + meta_infos.insert(feature.metainfo(j)); + } + (*features)[feature.name()] = meta_infos; + } + } + content::TestBrowserThread ui_thread_; int num_pending_; scoped_ptr<BrowserFeatureExtractor> extractor_; @@ -465,8 +485,10 @@ TEST_F(BrowserFeatureExtractorTest, BrowseFeatures) { GURL("https://bankofamerica.com"), content::PAGE_TRANSITION_GENERATED); - browse_info_->ips.insert("193.5.163.8"); - browse_info_->ips.insert("23.94.78.1"); + std::set<std::string> hosts; + hosts.insert("test.com"); + browse_info_->ips.insert(std::make_pair("193.5.163.8", hosts)); + browse_info_->ips.insert(std::make_pair("23.94.78.1", hosts)); EXPECT_CALL(*service_, IsBadIpAddress("193.5.163.8")).WillOnce(Return(true)); EXPECT_CALL(*service_, IsBadIpAddress("23.94.78.1")).WillOnce(Return(false)); @@ -520,4 +542,40 @@ TEST_F(BrowserFeatureExtractorTest, SafeBrowsingFeatures) { EXPECT_DOUBLE_EQ(1.0, features[features::kSafeBrowsingIsSubresource]); EXPECT_DOUBLE_EQ(2.0, features[features::kSafeBrowsingThreatType]); } + +TEST_F(BrowserFeatureExtractorTest, MalwareFeatures) { + ClientMalwareRequest request; + request.set_url("http://www.foo.com/"); + + std::set<std::string> bad_hosts; + bad_hosts.insert("bad.com"); + bad_hosts.insert("evil.com"); + browse_info_->ips.insert(std::make_pair("193.5.163.8", bad_hosts)); + browse_info_->ips.insert(std::make_pair("92.92.92.92", bad_hosts)); + std::set<std::string> good_hosts; + good_hosts.insert("ok.com"); + browse_info_->ips.insert(std::make_pair("23.94.78.1", good_hosts)); + EXPECT_CALL(*service_, IsBadIpAddress("193.5.163.8")).WillOnce(Return(true)); + EXPECT_CALL(*service_, IsBadIpAddress("92.92.92.92")).WillOnce(Return(true)); + EXPECT_CALL(*service_, IsBadIpAddress("23.94.78.1")).WillOnce(Return(false)); + + ExtractMalwareFeatures(&request); + std::map<std::string, std::set<std::string> > features; + GetMalwareFeatureMap(request, &features); + + EXPECT_EQ(2U, features.size()); + std::string feature_name = StringPrintf("%s%s", features::kBadIpFetch, + "193.5.163.8"); + EXPECT_TRUE(features.count(feature_name)); + std::set<std::string> hosts = features[feature_name]; + EXPECT_EQ(2U, hosts.size()); + EXPECT_TRUE(hosts.find("bad.com") != hosts.end()); + EXPECT_TRUE(hosts.find("evil.com") != hosts.end()); + feature_name = StringPrintf("%s%s", features::kBadIpFetch, "92.92.92.92"); + EXPECT_TRUE(features.count(feature_name)); + hosts = features[feature_name]; + EXPECT_EQ(2U, hosts.size()); + EXPECT_TRUE(hosts.find("bad.com") != hosts.end()); + EXPECT_TRUE(hosts.find("evil.com") != hosts.end()); +} } // 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 2529937..69d0e05 100644 --- a/chrome/browser/safe_browsing/client_side_detection_host.cc +++ b/chrome/browser/safe_browsing/client_side_detection_host.cc @@ -43,6 +43,9 @@ using content::WebContents; namespace safe_browsing { +const int ClientSideDetectionHost::kMaxHostsPerIP = 20; +const int ClientSideDetectionHost::kMaxIPsPerBrowse = 200; + namespace { void EmptyUrlCheckCallback(bool processed) { @@ -376,22 +379,32 @@ void ClientSideDetectionHost::OnPhishingDetectionDone( !weak_factory_.HasWeakPtrs() && browse_info_.get() && verdict->ParseFromString(verdict_str) && - verdict->IsInitialized() && - // We only send the verdict to the server if the verdict is phishing or if - // a SafeBrowsing interstitial was already shown for this site. E.g., a - // malware or phishing interstitial was shown but the user clicked - // through. - (verdict->is_phishing() || DidShowSBInterstitial())) { - if (DidShowSBInterstitial()) { - browse_info_->unsafe_resource.reset(unsafe_resource_.release()); - } - // Start browser-side feature extraction. Once we're done it will send - // the client verdict request. - feature_extractor_->ExtractFeatures( + verdict->IsInitialized()) { + scoped_ptr<ClientMalwareRequest> malware_verdict(new ClientMalwareRequest); + // Start browser-side malware feature extraction. Once we're done it will + // send the malware client verdict request. + malware_verdict->set_url(verdict->url()); + feature_extractor_->ExtractMalwareFeatures( browse_info_.get(), - verdict.release(), - base::Bind(&ClientSideDetectionHost::FeatureExtractionDone, - weak_factory_.GetWeakPtr())); + malware_verdict.get()); + MalwareFeatureExtractionDone(malware_verdict.Pass()); + + // We only send phishing verdict to the server if the verdict is phishing or + // if a SafeBrowsing interstitial was already shown for this site. E.g., a + // malware or phishing interstitial was shown but the user clicked + // through. + if (verdict->is_phishing() || DidShowSBInterstitial()) { + if (DidShowSBInterstitial()) { + browse_info_->unsafe_resource.reset(unsafe_resource_.release()); + } + // Start browser-side feature extraction. Once we're done it will send + // the client verdict request. + feature_extractor_->ExtractFeatures( + browse_info_.get(), + verdict.release(), + base::Bind(&ClientSideDetectionHost::FeatureExtractionDone, + weak_factory_.GetWeakPtr())); + } } browse_info_.reset(); } @@ -446,6 +459,25 @@ void ClientSideDetectionHost::FeatureExtractionDone( callback); } +void ClientSideDetectionHost::MalwareFeatureExtractionDone( + scoped_ptr<ClientMalwareRequest> request) { + if (!request) { + DLOG(FATAL) << "Invalid request object in MalwareFeatureExtractionDone"; + return; + } + VLOG(2) << "Malware Feature extraction done for URL: " << request->url() + << ", with features count:" << request->feature_map_size(); + + // Send ping if there is matching features. + if (request->feature_map_size() > 0) { + VLOG(1) << "Start sending client malware request."; + ClientSideDetectionService::ClientReportMalwareRequestCallback callback; + csd_service_->SendClientReportMalwareRequest( + request.release(), // The service takes ownership of the request object + callback); // no action after request sent for now + } +} + void ClientSideDetectionHost::Observe( int type, const content::NotificationSource& source, @@ -455,7 +487,16 @@ void ClientSideDetectionHost::Observe( const ResourceRequestDetails* req = content::Details<ResourceRequestDetails>( details).ptr(); if (req && browse_info_.get()) { - browse_info_->ips.insert(req->socket_address.host()); + std::string ip = req->socket_address.host(); + IPHostMap::iterator it = browse_info_->ips.find(ip); + if (it == browse_info_->ips.end() + && int(browse_info_->ips.size()) < kMaxIPsPerBrowse) { + std::set<std::string> hosts; + hosts.insert(req->url.host()); + browse_info_->ips.insert(make_pair(req->socket_address.host(), hosts)); + } else if (int(it->second.size()) < kMaxHostsPerIP) { + it->second.insert(req->url.host()); + } } } diff --git a/chrome/browser/safe_browsing/client_side_detection_host.h b/chrome/browser/safe_browsing/client_side_detection_host.h index a567439..fa02863 100644 --- a/chrome/browser/safe_browsing/client_side_detection_host.h +++ b/chrome/browser/safe_browsing/client_side_detection_host.h @@ -74,6 +74,8 @@ class ClientSideDetectionHost : public content::WebContentsObserver, // This method is responsible for deleting the request object. Called on // the UI thread. void FeatureExtractionDone(bool success, ClientPhishingRequest* request); + // Function to be called when the browser malware feature extractor is done. + void MalwareFeatureExtractionDone(scoped_ptr<ClientMalwareRequest> request); // From NotificationObserver. Called when a notification comes in. This // method is called in the UI thread. @@ -119,6 +121,11 @@ class ClientSideDetectionHost : public content::WebContentsObserver, // Handles registering notifications with the NotificationService. content::NotificationRegistrar registrar_; + // Max number of ips we save for each browse + static const int kMaxIPsPerBrowse; + // Max number of hosts we report for each malware IP. + static const int kMaxHostsPerIP; + base::WeakPtrFactory<ClientSideDetectionHost> weak_factory_; // Unique page ID of the most recent unsafe site that was loaded in this tab 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 00ab995..0f29dda 100644 --- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc +++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc @@ -61,6 +61,11 @@ MATCHER_P(PartiallyEqualVerdict, other, "") { other.is_phishing() == arg.is_phishing()); } +MATCHER_P(PartiallyEqualMalwareVerdict, other, "") { + return (other.url() == arg.url() && + other.feature_map_size() == arg.feature_map_size()); +} + // Test that the callback is NULL when the verdict is not phishing. MATCHER(CallbackIsNull, "") { return arg.is_null(); @@ -90,6 +95,9 @@ class MockClientSideDetectionService : public ClientSideDetectionService { MOCK_METHOD2(SendClientReportPhishingRequest, void(ClientPhishingRequest*, const ClientReportPhishingRequestCallback&)); + MOCK_METHOD2(SendClientReportMalwareRequest, + void(ClientMalwareRequest*, + const ClientReportMalwareRequestCallback&)); MOCK_CONST_METHOD1(IsPrivateIPAddress, bool(const std::string&)); MOCK_METHOD2(GetValidCachedResult, bool(const GURL&, bool*)); MOCK_METHOD1(IsInCache, bool(const GURL&)); @@ -157,6 +165,10 @@ class MockBrowserFeatureExtractor : public BrowserFeatureExtractor { void(const BrowseInfo* info, ClientPhishingRequest*, const BrowserFeatureExtractor::DoneCallback&)); + + MOCK_METHOD2(ExtractMalwareFeatures, + void(const BrowseInfo* info, + ClientMalwareRequest*)); }; // Helper function which quits the UI message loop from the IO message loop. @@ -610,6 +622,107 @@ TEST_F(ClientSideDetectionHostTest, EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get())); } +TEST_F(ClientSideDetectionHostTest, + OnPhishingDetectionDoneVerdictNotPhishingNotMalwareIP) { + // Case 7: renderer sends a verdict string that isn't phishing and not matches + // malware bad IP list + MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor( + web_contents(), + csd_service_.get()); + SetFeatureExtractor(mock_extractor); // The host class takes ownership. + + ClientPhishingRequest verdict; + verdict.set_url("http://not-phishing.com/"); + verdict.set_client_score(0.1f); + verdict.set_is_phishing(false); + + ClientMalwareRequest malware_verdict; + malware_verdict.set_url("http://not-phishing.com/"); + + EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _)) + .WillOnce(SetArgumentPointee<1>(malware_verdict)); + EXPECT_CALL(*csd_service_, + SendClientReportMalwareRequest(_, _)).Times(0); + EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0); + + OnPhishingDetectionDone(verdict.SerializeAsString()); + EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor)); +} + +TEST_F(ClientSideDetectionHostTest, + OnPhishingDetectionDoneVerdictNotPhishingButMalwareIP) { + // Case 8: renderer sends a verdict string that isn't phishing but matches + // malware bad IP list + MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor( + web_contents(), + csd_service_.get()); + SetFeatureExtractor(mock_extractor); // The host class takes ownership. + + ClientPhishingRequest verdict; + verdict.set_url("http://not-phishing.com/"); + verdict.set_client_score(0.1f); + verdict.set_is_phishing(false); + + ClientMalwareRequest malware_verdict; + malware_verdict.set_url("http://not-phishing.com/"); + ClientMalwareRequest::Feature* feature = malware_verdict.add_feature_map(); + feature->set_name("malwareip1.2.3.4"); + feature->set_value(1.0); + feature->add_metainfo("badip.com"); + + EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _)) + .WillOnce(SetArgumentPointee<1>(malware_verdict)); + EXPECT_CALL(*csd_service_, + SendClientReportMalwareRequest( + Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _)); + EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0); + + OnPhishingDetectionDone(verdict.SerializeAsString()); + EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor)); +} + +TEST_F(ClientSideDetectionHostTest, + OnPhishingDetectionDoneVerdictPhishingAndMalwareIP) { + // Case 9: renderer sends a verdict string that is phishing and matches + // malware bad IP list + MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor( + web_contents(), + csd_service_.get()); + SetFeatureExtractor(mock_extractor); // The host class takes ownership. + + ClientSideDetectionService::ClientReportPhishingRequestCallback cb; + ClientPhishingRequest verdict; + verdict.set_url("http://not-phishing.com/"); + verdict.set_client_score(0.1f); + verdict.set_is_phishing(true); + + ClientMalwareRequest malware_verdict; + malware_verdict.set_url("http://not-phishing.com/"); + ClientMalwareRequest::Feature* feature = malware_verdict.add_feature_map(); + feature->set_name("malwareip1.2.3.4"); + feature->set_value(1.0); + feature->add_metainfo("badip.com"); + + EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _)) + .WillOnce(SetArgumentPointee<1>(malware_verdict)); + EXPECT_CALL(*csd_service_, + SendClientReportMalwareRequest( + Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _)); + + EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)) + .WillOnce(DoAll(DeleteArg<1>(), + InvokeCallbackArgument<2>(true, &verdict))); + + EXPECT_CALL(*csd_service_, + SendClientReportPhishingRequest( + Pointee(PartiallyEqualVerdict(verdict)), _)) + .WillOnce(SaveArg<1>(&cb)); + OnPhishingDetectionDone(verdict.SerializeAsString()); + EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor)); + EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get())); + ASSERT_FALSE(cb.is_null()); +} + TEST_F(ClientSideDetectionHostTest, NavigationCancelsShouldClassifyUrl) { // Test that canceling pending should classify requests works as expected. diff --git a/chrome/browser/safe_browsing/client_side_detection_service.cc b/chrome/browser/safe_browsing/client_side_detection_service.cc index f8d6067..7c805c2 100644 --- a/chrome/browser/safe_browsing/client_side_detection_service.cc +++ b/chrome/browser/safe_browsing/client_side_detection_service.cc @@ -52,6 +52,8 @@ const int ClientSideDetectionService::kPositiveCacheIntervalMinutes = 30; const char ClientSideDetectionService::kClientReportPhishingUrl[] = "https://sb-ssl.google.com/safebrowsing/clientreport/phishing"; +const char ClientSideDetectionService::kClientReportMalwareUrl[] = + "https://sb-ssl.google.com/safebrowsing/clientreport/malware-check"; const char ClientSideDetectionService::kClientModelUrl[] = "https://ssl.gstatic.com/safebrowsing/csd/client_model_v4.pb"; @@ -78,6 +80,9 @@ ClientSideDetectionService::~ClientSideDetectionService() { STLDeleteContainerPairPointers(client_phishing_reports_.begin(), client_phishing_reports_.end()); client_phishing_reports_.clear(); + STLDeleteContainerPairPointers(client_malware_reports_.begin(), + client_malware_reports_.end()); + client_malware_reports_.clear(); } // static @@ -120,6 +125,16 @@ void ClientSideDetectionService::SetEnabledAndRefreshState(bool enabled) { STLDeleteContainerPairPointers(client_phishing_reports_.begin(), client_phishing_reports_.end()); client_phishing_reports_.clear(); + for (std::map<const net::URLFetcher*, ClientReportInfo*>::iterator it = + client_malware_reports_.begin(); + it != client_malware_reports_.end(); ++it) { + ClientReportInfo* info = it->second; + if (!info->callback.is_null()) + info->callback.Run(info->phishing_url, false); + } + STLDeleteContainerPairPointers(client_malware_reports_.begin(), + client_malware_reports_.end()); + client_malware_reports_.clear(); cache_.clear(); } } @@ -134,6 +149,16 @@ void ClientSideDetectionService::SendClientReportPhishingRequest( weak_factory_.GetWeakPtr(), verdict, callback)); } +void ClientSideDetectionService::SendClientReportMalwareRequest( + ClientMalwareRequest* verdict, + const ClientReportMalwareRequestCallback& callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&ClientSideDetectionService::StartClientReportMalwareRequest, + weak_factory_.GetWeakPtr(), verdict, callback)); +} + bool ClientSideDetectionService::IsPrivateIPAddress( const std::string& ip_address) const { net::IPAddressNumber ip_number; @@ -195,6 +220,11 @@ void ClientSideDetectionService::OnURLFetchComplete( HandlePhishingVerdict( source, source->GetURL(), source->GetStatus(), source->GetResponseCode(), source->GetCookies(), data); + } else if (client_malware_reports_.find(source) != + client_malware_reports_.end()) { + HandleMalwareVerdict( + source, source->GetURL(), source->GetStatus(), + source->GetResponseCode(), source->GetCookies(), data); } else { NOTREACHED(); } @@ -305,7 +335,8 @@ void ClientSideDetectionService::StartClientReportPhishingRequest( } net::URLFetcher* fetcher = net::URLFetcher::Create( - 0 /* ID used for testing */, GURL(GetClientReportPhishingUrl()), + 0 /* ID used for testing */, + GURL(GetClientReportUrl(kClientReportPhishingUrl)), net::URLFetcher::POST, this); // Remember which callback and URL correspond to the current fetcher object. @@ -323,6 +354,45 @@ void ClientSideDetectionService::StartClientReportPhishingRequest( phishing_report_times_.push(base::Time::Now()); } +void ClientSideDetectionService::StartClientReportMalwareRequest( + ClientMalwareRequest* verdict, + const ClientReportMalwareRequestCallback& callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + scoped_ptr<ClientMalwareRequest> request(verdict); + + if (!enabled_) { + if (!callback.is_null()) + callback.Run(GURL(request->url()), false); + return; + } + + std::string request_data; + if (!request->SerializeToString(&request_data)) { + UMA_HISTOGRAM_COUNTS("SBClientMalware.RequestNotSerialized", 1); + VLOG(1) << "Unable to serialize the CSD request. Proto file changed?"; + if (!callback.is_null()) + callback.Run(GURL(request->url()), false); + return; + } + + net::URLFetcher* fetcher = net::URLFetcher::Create( + 0 /* ID used for testing */, + GURL(GetClientReportUrl(kClientReportMalwareUrl)), + net::URLFetcher::POST, this); + + // Remember which callback and URL correspond to the current fetcher object. + // TODO: need to modify to malware specific code + ClientReportInfo* info = new ClientReportInfo; + info->callback = callback; + info->phishing_url = GURL(request->url()); + client_malware_reports_[fetcher] = info; + + fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE); + fetcher->SetRequestContext(request_context_getter_.get()); + fetcher->SetUploadData("application/octet-stream", request_data); + fetcher->Start(); +} + void ClientSideDetectionService::HandleModelResponse( const net::URLFetcher* source, const GURL& url, @@ -391,6 +461,30 @@ void ClientSideDetectionService::HandlePhishingVerdict( delete source; } +void ClientSideDetectionService::HandleMalwareVerdict( + const net::URLFetcher* source, + const GURL& url, + const net::URLRequestStatus& status, + int response_code, + const net::ResponseCookies& cookies, + const std::string& data) { + ClientMalwareResponse response; + scoped_ptr<ClientReportInfo> info(client_malware_reports_[source]); + bool should_blacklist = false; + if (status.is_success() && net::HTTP_OK == response_code && + response.ParseFromString(data)) { + should_blacklist = response.blacklist(); + } else { + DLOG(ERROR) << "Unable to get the server verdict for URL: " + << info->phishing_url << " status: " << status.status() << " " + << "response_code:" << response_code; + } + if (!info->callback.is_null()) + info->callback.Run(info->phishing_url, should_blacklist); + client_malware_reports_.erase(source); + delete source; +} + bool ClientSideDetectionService::IsInCache(const GURL& url) { UpdateCache(); @@ -536,8 +630,9 @@ bool ClientSideDetectionService::ModelHasValidHashIds( } // static -std::string ClientSideDetectionService::GetClientReportPhishingUrl() { - std::string url = kClientReportPhishingUrl; +std::string ClientSideDetectionService::GetClientReportUrl( + const std::string& report_url) { + std::string url = report_url; std::string api_key = google_apis::GetAPIKey(); if (!api_key.empty()) { base::StringAppendF(&url, "?key=%s", diff --git a/chrome/browser/safe_browsing/client_side_detection_service.h b/chrome/browser/safe_browsing/client_side_detection_service.h index 77efd13..f6fe23e 100644 --- a/chrome/browser/safe_browsing/client_side_detection_service.h +++ b/chrome/browser/safe_browsing/client_side_detection_service.h @@ -52,6 +52,7 @@ typedef std::vector<std::string> ResponseCookies; } // namespace net namespace safe_browsing { +class ClientMalwareRequest; class ClientPhishingRequest; class ClientPhishingResponse; class ClientSideModel; @@ -61,6 +62,7 @@ class ClientSideDetectionService : public net::URLFetcherDelegate, public: // void(GURL phishing_url, bool is_phishing). typedef base::Callback<void(GURL, bool)> ClientReportPhishingRequestCallback; + typedef base::Callback<void(GURL, bool)> ClientReportMalwareRequestCallback; virtual ~ClientSideDetectionService(); @@ -104,6 +106,11 @@ class ClientSideDetectionService : public net::URLFetcherDelegate, ClientPhishingRequest* verdict, const ClientReportPhishingRequestCallback& callback); + // Similar to above one, instead send ClientMalwareRequest + virtual void SendClientReportMalwareRequest( + ClientMalwareRequest* verdict, + const ClientReportMalwareRequestCallback& callback); + // Returns true if the given IP address string falls within a private // (unroutable) network block. Pages which are hosted on these IP addresses // are exempt from client-side phishing detection. This is called by the @@ -190,6 +197,7 @@ class ClientSideDetectionService : public net::URLFetcherDelegate, typedef std::map<std::string /* subnet mask */, std::set<std::string /* hashed subnet */> > BadSubnetMap; + static const char kClientReportMalwareUrl[]; static const char kClientReportPhishingUrl[]; static const char kClientModelUrl[]; static const size_t kMaxModelSizeBytes; @@ -206,6 +214,10 @@ class ClientSideDetectionService : public net::URLFetcherDelegate, ClientPhishingRequest* verdict, const ClientReportPhishingRequestCallback& callback); + void StartClientReportMalwareRequest( + ClientMalwareRequest* verdict, + const ClientReportMalwareRequestCallback& callback); + // Called by OnURLFetchComplete to handle the response from fetching the // model. void HandleModelResponse(const net::URLFetcher* source, @@ -224,6 +236,15 @@ class ClientSideDetectionService : public net::URLFetcherDelegate, const net::ResponseCookies& cookies, const std::string& data); + // Called by OnURLFetchComplete to handle the server response from + // sending the client-side malware request. + void HandleMalwareVerdict(const net::URLFetcher* source, + const GURL& url, + const net::URLRequestStatus& status, + int response_code, + const net::ResponseCookies& cookies, + const std::string& data); + // Invalidate cache results which are no longer useful. void UpdateCache(); @@ -252,7 +273,7 @@ class ClientSideDetectionService : public net::URLFetcherDelegate, static bool ModelHasValidHashIds(const ClientSideModel& model); // Returns the URL that will be used for phishing requests. - static std::string GetClientReportPhishingUrl(); + static std::string GetClientReportUrl(const std::string& report_url); // Whether the service is running or not. When the service is not running, // it won't download the model nor report detected phishing URLs. @@ -268,6 +289,8 @@ class ClientSideDetectionService : public net::URLFetcherDelegate, struct ClientReportInfo; std::map<const net::URLFetcher*, ClientReportInfo*> client_phishing_reports_; + std::map<const net::URLFetcher*, ClientReportInfo*> + client_malware_reports_; // Cache of completed requests. Used to satisfy requests for the same urls // as long as the next request falls within our caching window (which is 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 232c885..f5fd98b 100644 --- a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc +++ b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc @@ -93,6 +93,18 @@ class ClientSideDetectionServiceTest : public testing::Test { return is_phishing_; } + bool SendClientReportMalwareRequest(const GURL& url) { + scoped_ptr<ClientMalwareRequest> request(new ClientMalwareRequest()); + request->set_url(url.spec()); + csd_service_->SendClientReportMalwareRequest( + request.release(), + base::Bind(&ClientSideDetectionServiceTest::SendMalwareRequestDone, + base::Unretained(this))); + phishing_url_ = url; + msg_loop_.Run(); // Waits until callback is called. + return is_malware_; + } + void SetModelFetchResponse(std::string response_data, bool success) { factory_->SetFakeResponse(ClientSideDetectionService::kClientModelUrl, response_data, success); @@ -101,7 +113,16 @@ class ClientSideDetectionServiceTest : public testing::Test { void SetClientReportPhishingResponse(std::string response_data, bool success) { factory_->SetFakeResponse( - ClientSideDetectionService::GetClientReportPhishingUrl(), + ClientSideDetectionService::GetClientReportUrl( + ClientSideDetectionService::kClientReportPhishingUrl), + response_data, success); + } + + void SetClientReportMalwareResponse(std::string response_data, + bool success) { + factory_->SetFakeResponse( + ClientSideDetectionService::GetClientReportUrl( + ClientSideDetectionService::kClientReportMalwareUrl), response_data, success); } @@ -200,11 +221,17 @@ class ClientSideDetectionServiceTest : public testing::Test { msg_loop_.Quit(); } + void SendMalwareRequestDone(GURL url, bool is_malware) { + ASSERT_EQ(phishing_url_, url); + is_malware_ = is_malware; + msg_loop_.Quit(); + } scoped_ptr<content::TestBrowserThread> browser_thread_; scoped_ptr<content::TestBrowserThread> file_thread_; GURL phishing_url_; bool is_phishing_; + bool is_malware_; }; TEST_F(ClientSideDetectionServiceTest, FetchModelTest) { @@ -381,6 +408,34 @@ TEST_F(ClientSideDetectionServiceTest, SendClientReportPhishingRequest) { EXPECT_FALSE(csd_service_->IsInCache(second_url)); } +TEST_F(ClientSideDetectionServiceTest, SendClientReportMalwareRequest) { + SetModelFetchResponse("bogus model", true /* success */); + csd_service_.reset(ClientSideDetectionService::Create(NULL)); + csd_service_->SetEnabledAndRefreshState(true); + GURL url("http://a.com/"); + + // Invalid response body from the server. + SetClientReportMalwareResponse("invalid proto response", true /* success */); + EXPECT_FALSE(SendClientReportMalwareRequest(url)); + + // Normal behavior. + ClientMalwareResponse response; + response.set_blacklist(true); + SetClientReportMalwareResponse(response.SerializeAsString(), true); + EXPECT_TRUE(SendClientReportMalwareRequest(url)); + + // This request will fail + response.set_blacklist(false); + SetClientReportMalwareResponse(response.SerializeAsString(), + false /* success */); + EXPECT_FALSE(SendClientReportMalwareRequest(url)); + + // server blacklist decision is false, and response is succesful + response.set_blacklist(false); + SetClientReportMalwareResponse(response.SerializeAsString(), true); + EXPECT_FALSE(SendClientReportMalwareRequest(url)); +} + TEST_F(ClientSideDetectionServiceTest, GetNumReportTest) { SetModelFetchResponse("bogus model", true /* success */); csd_service_.reset(ClientSideDetectionService::Create(NULL)); |