summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorlzheng@chromium.org <lzheng@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-12-22 21:07:05 +0000
committerlzheng@chromium.org <lzheng@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-12-22 21:07:05 +0000
commit9fc1e6c183707ebde09cb0057f528646f309b32d (patch)
tree4e45db2ebbca3aea8dd214855566ff8f0fc920c5
parent6eb56b870387d485303fd0c5f08f1849df4561a3 (diff)
downloadchromium_src-9fc1e6c183707ebde09cb0057f528646f309b32d.zip
chromium_src-9fc1e6c183707ebde09cb0057f528646f309b32d.tar.gz
chromium_src-9fc1e6c183707ebde09cb0057f528646f309b32d.tar.bz2
Add support for safebrowsing download protection. It is currently under flag --safebrowsing-download-protection.
TEST=safe_browsing_service_browsertest.cc BUG=60822 Review URL: http://codereview.chromium.org/5141006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69979 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/renderer_host/download_resource_handler.cc67
-rw-r--r--chrome/browser/renderer_host/download_resource_handler.h27
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_service.cc39
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_service.h5
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc125
5 files changed, 227 insertions, 36 deletions
diff --git a/chrome/browser/renderer_host/download_resource_handler.cc b/chrome/browser/renderer_host/download_resource_handler.cc
index 6e34a30..f806406 100644
--- a/chrome/browser/renderer_host/download_resource_handler.cc
+++ b/chrome/browser/renderer_host/download_resource_handler.cc
@@ -7,6 +7,8 @@
#include <string>
#include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/stats_counters.h"
#include "base/stringprintf.h"
#include "chrome/browser/browser_thread.h"
#include "chrome/browser/download/download_item.h"
@@ -41,7 +43,8 @@ DownloadResourceHandler::DownloadResourceHandler(
save_info_(save_info),
buffer_(new DownloadBuffer),
rdh_(rdh),
- is_paused_(false) {
+ is_paused_(false),
+ url_check_pending_(false) {
}
bool DownloadResourceHandler::OnUploadProgress(int request_id,
@@ -59,11 +62,55 @@ bool DownloadResourceHandler::OnRequestRedirected(int request_id,
return true;
}
+// Callback when the result of checking a download URL is known.
+// TODO(lzheng): We should create a information bar with buttons to ask
+// if users want to proceed when the download is malicious.
+void DownloadResourceHandler::OnDownloadUrlCheckResult(
+ const GURL& url, SafeBrowsingService::UrlCheckResult result) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ DCHECK(url_check_pending_);
+
+ UMA_HISTOGRAM_TIMES("SB2.DownloadUrlCheckDuration",
+ base::TimeTicks::Now() - download_start_time_);
+
+ if (result == SafeBrowsingService::BINARY_MALWARE) {
+ // TODO(lzheng): More UI work to show warnings properly on download shelf.
+ DLOG(WARNING) << "This url leads to a malware downloading: "
+ << url.spec();
+ UpdateDownloadUrlCheckStats(DOWNLOAD_URL_CHECKS_MALWARE);
+ }
+
+ url_check_pending_ = false;
+ // Note: Release() should be the last line in this call. It is for
+ // the AddRef in CheckSafeBrowsing.
+ Release();
+}
+
+// Send the download creation information to the download thread.
+void DownloadResourceHandler::StartDownloadUrlCheck() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ AddRef();
+ if (!rdh_->safe_browsing_service()->CheckDownloadUrl(url_, this)) {
+ url_check_pending_ = true;
+ UpdateDownloadUrlCheckStats(DOWNLOAD_URL_CHECKS_TOTAL);
+ // Note: in this case, the AddRef will be balanced in
+ // "OnDownloadUrlCheckResult" or "OnRequestClosed".
+ } else {
+ // Immediately release the AddRef() at the beginning of this function
+ // since no more callbacks will happen.
+ Release();
+ DVLOG(1) << "url: " << url_.spec() << " is safe to download.";
+ }
+}
+
// Send the download creation information to the download thread.
bool DownloadResourceHandler::OnResponseStarted(int request_id,
ResourceResponse* response) {
VLOG(20) << __FUNCTION__ << "()" << DebugString()
<< " request_id = " << request_id;
+ DCHECK(!url_check_pending_);
+ download_start_time_ = base::TimeTicks::Now();
+ StartDownloadUrlCheck();
std::string content_disposition;
request_->GetResponseHeaderByName("content-disposition",
&content_disposition);
@@ -183,6 +230,17 @@ bool DownloadResourceHandler::OnResponseCompleted(
}
void DownloadResourceHandler::OnRequestClosed() {
+ UMA_HISTOGRAM_TIMES("SB2.DownloadDuration",
+ base::TimeTicks::Now() - download_start_time_);
+ if (url_check_pending_) {
+ DVLOG(1) << "Cancel pending download url checking request: " << this;
+ rdh_->safe_browsing_service()->CancelCheck(this);
+ UpdateDownloadUrlCheckStats(DOWNLOAD_URL_CHECKS_CANCELED);
+ url_check_pending_ = false;
+ // Balance the AddRef() from StartDownloadUrlCheck() which would usually be
+ // balanced by OnDownloadUrlCheckResult().
+ Release();
+ }
}
// If the content-length header is not present (or contains something other
@@ -249,3 +307,10 @@ std::string DownloadResourceHandler::DebugString() const {
render_view_id_,
save_info_.file_path.value().c_str());
}
+
+void DownloadResourceHandler::UpdateDownloadUrlCheckStats(
+ SBStatsType stat_type) {
+ UMA_HISTOGRAM_ENUMERATION("SB2.DownloadUrlChecks",
+ stat_type,
+ DOWNLOAD_URL_CHECKS_MAX);
+}
diff --git a/chrome/browser/renderer_host/download_resource_handler.h b/chrome/browser/renderer_host/download_resource_handler.h
index 3fb6961..5a66f66 100644
--- a/chrome/browser/renderer_host/download_resource_handler.h
+++ b/chrome/browser/renderer_host/download_resource_handler.h
@@ -12,6 +12,7 @@
#include "chrome/browser/download/download_file.h"
#include "chrome/browser/renderer_host/global_request_id.h"
#include "chrome/browser/renderer_host/resource_handler.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
class DownloadFileManager;
class ResourceDispatcherHost;
@@ -22,7 +23,8 @@ class URLRequest;
} // namespace net
// Forwards data to the download thread.
-class DownloadResourceHandler : public ResourceHandler {
+class DownloadResourceHandler : public ResourceHandler,
+ public SafeBrowsingService::Client {
public:
DownloadResourceHandler(ResourceDispatcherHost* rdh,
int render_process_host_id,
@@ -70,10 +72,31 @@ class DownloadResourceHandler : public ResourceHandler {
std::string DebugString() const;
private:
+ // Enumerate for histogramming purposes. DO NOT CHANGE THE
+ // ORDERING OF THESE VALUES.
+ enum SBStatsType {
+ DOWNLOAD_URL_CHECKS_TOTAL,
+ DOWNLOAD_URL_CHECKS_CANCELED,
+ DOWNLOAD_URL_CHECKS_MALWARE,
+
+ // Memory space for histograms is determined by the max. ALWAYS
+ // ADD NEW VALUES BEFORE THIS ONE.
+ DOWNLOAD_URL_CHECKS_MAX
+ };
+
~DownloadResourceHandler();
void StartPauseTimer();
+ void StartDownloadUrlCheck();
+
+ // Called when the result of checking a download URL is known.
+ void OnDownloadUrlCheckResult(const GURL& url,
+ SafeBrowsingService::UrlCheckResult result);
+
+ // A helper function that updates UMA for download url checks.
+ static void UpdateDownloadUrlCheckStats(SBStatsType stat_type);
+
int download_id_;
GlobalRequestID global_id_;
int render_view_id_;
@@ -89,6 +112,8 @@ class DownloadResourceHandler : public ResourceHandler {
ResourceDispatcherHost* rdh_;
bool is_paused_;
base::OneShotTimer<DownloadResourceHandler> pause_timer_;
+ bool url_check_pending_;
+ base::TimeTicks download_start_time_; // used to collect stats.
static const int kReadBufSize = 32768; // bytes
static const size_t kLoadsToWrite = 100; // number of data buffers queued
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 657778a..9e24b03 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -136,42 +136,35 @@ bool SafeBrowsingService::CanCheckUrl(const GURL& url) const {
}
void SafeBrowsingService::CheckDownloadUrlDone(
- Client* client, const GURL& url, UrlCheckResult result) {
+ SafeBrowsingCheck* check, UrlCheckResult result) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
DCHECK(enable_download_protection_);
VLOG(1) << "CheckDownloadUrlDone: " << result;
- client->OnSafeBrowsingResult(url, result);
+
+ if (checks_.find(check) == checks_.end() || !check->client)
+ return;
+ check->client->OnSafeBrowsingResult(check->url, result);
+ checks_.erase(check);
}
-void SafeBrowsingService::CheckDownloadUrlOnSBThread(const GURL& url,
- Client* client) {
+void SafeBrowsingService::CheckDownloadUrlOnSBThread(SafeBrowsingCheck* check) {
DCHECK_EQ(MessageLoop::current(), safe_browsing_thread_->message_loop());
DCHECK(enable_download_protection_);
std::vector<SBPrefix> prefix_hits;
- if (!database_->ContainsDownloadUrl(url, &prefix_hits)) {
+ if (!database_->ContainsDownloadUrl(check->url, &prefix_hits)) {
// Good, we don't have hash for this url prefix.
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
NewRunnableMethod(this,
&SafeBrowsingService::CheckDownloadUrlDone,
- client, url, URL_SAFE));
+ check, URL_SAFE));
return;
}
- // Now, we need to fetch the url from the safebrowsing backends.
- // Needs to be asynchronous, since we could be in the constructor of a
- // ResourceDispatcherHost event handler which can't pause there.
- SafeBrowsingCheck* check = new SafeBrowsingCheck();
-
- check->url = url;
- check->client = client;
- check->result = URL_SAFE;
check->need_get_hash = true;
check->prefix_hits.swap(prefix_hits);
- checks_.insert(check);
-
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
NewRunnableMethod(this, &SafeBrowsingService::OnCheckDone, check));
@@ -183,8 +176,17 @@ bool SafeBrowsingService::CheckDownloadUrl(const GURL& url,
if (!enabled_ || !enable_download_protection_)
return true;
+ // We need to check the database for url prefix, and later may fetch the url
+ // from the safebrowsing backends. These need to be asynchronous.
+ SafeBrowsingCheck* check = new SafeBrowsingCheck();
+
+ check->url = url;
+ check->client = client;
+ check->result = URL_SAFE;
+ checks_.insert(check);
safe_browsing_thread_->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
- this, &SafeBrowsingService::CheckDownloadUrlOnSBThread, url, client));
+ this, &SafeBrowsingService::CheckDownloadUrlOnSBThread, check));
+
return false;
}
@@ -702,8 +704,7 @@ void SafeBrowsingService::DatabaseLoadComplete() {
}
void SafeBrowsingService::HandleChunkForDatabase(
- const std::string& list_name,
- SBChunkList* chunks) {
+ const std::string& list_name, SBChunkList* chunks) {
DCHECK_EQ(MessageLoop::current(), safe_browsing_thread_->message_loop());
if (chunks) {
GetDatabase()->InsertChunks(list_name, *chunks);
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index abade47..e87fc9c 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -323,11 +323,10 @@ class SafeBrowsingService
// Invoked by CheckDownloadUrl. It checks the download URL on
// safe_browsing_thread_.
- void CheckDownloadUrlOnSBThread(const GURL& url, Client* client);
+ void CheckDownloadUrlOnSBThread(SafeBrowsingCheck* check);
// Call the Client's callback in IO thread after CheckDownloadUrl finishes.
- void CheckDownloadUrlDone(Client* client, const GURL& url,
- UrlCheckResult result);
+ void CheckDownloadUrlDone(SafeBrowsingCheck* check, UrlCheckResult result);
// The factory used to instanciate a SafeBrowsingService object.
// Useful for tests, so they can provide their own implementation of
diff --git a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
index c495481..0b44a250 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
@@ -7,9 +7,12 @@
// service.
#include "base/command_line.h"
+#include "base/metrics/histogram.h"
+#include "base/ref_counted.h"
#include "base/sha2.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/browser_thread.h"
+#include "chrome/browser/renderer_host/resource_dispatcher_host.h"
#include "chrome/browser/safe_browsing/protocol_manager.h"
#include "chrome/browser/safe_browsing/safe_browsing_database.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
@@ -21,6 +24,9 @@
#include "chrome/test/in_process_browser_test.h"
#include "chrome/test/ui_test_utils.h"
+using base::Histogram;
+using base::StatisticsRecorder;
+
namespace {
// A SafeBrowingDatabase class that allows us to inject the malicious URLs.
@@ -47,19 +53,17 @@ class TestSafeBrowsingDatabase : public SafeBrowsingDatabase {
std::vector<SBPrefix>* prefix_hits,
std::vector<SBFullHashResult>* full_hits,
base::Time last_update) {
- base::hash_map<std::string, Hits>::const_iterator
- badurls_it = badurls_.find(url.spec());
- if (badurls_it == badurls_.end())
- return false;
- *prefix_hits = badurls_it->second.prefix_hits;
- *full_hits = badurls_it->second.full_hits;
- return true;
+ return ContainsUrl(safe_browsing_util::kMalwareList,
+ safe_browsing_util::kPhishingList,
+ url, prefix_hits, full_hits);
}
virtual bool ContainsDownloadUrl(const GURL& url,
std::vector<SBPrefix>* prefix_hits) {
- ADD_FAILURE() << "Not implemented.";
- return false;
+ std::vector<SBFullHashResult> full_hits;
+ return ContainsUrl(safe_browsing_util::kBinUrlList,
+ safe_browsing_util::kBinHashList,
+ url, prefix_hits, &full_hits);
}
virtual bool UpdateStarted(std::vector<SBListChunkRanges>* lists) {
@@ -84,16 +88,42 @@ class TestSafeBrowsingDatabase : public SafeBrowsingDatabase {
// Fill up the database with test URL.
void AddUrl(const GURL& url,
+ const std::string& list_name,
const std::vector<SBPrefix>& prefix_hits,
const std::vector<SBFullHashResult>& full_hits) {
+ badurls_[url.spec()].list_name = list_name;
badurls_[url.spec()].prefix_hits = prefix_hits;
badurls_[url.spec()].full_hits = full_hits;
}
+
private:
struct Hits {
+ std::string list_name;
std::vector<SBPrefix> prefix_hits;
std::vector<SBFullHashResult> full_hits;
};
+
+ bool ContainsUrl(const std::string& list_name0,
+ const std::string& list_name1,
+ const GURL& url,
+ std::vector<SBPrefix>* prefix_hits,
+ std::vector<SBFullHashResult>* full_hits) {
+ base::hash_map<std::string, Hits>::const_iterator
+ badurls_it = badurls_.find(url.spec());
+
+ if (badurls_it == badurls_.end())
+ return false;
+
+ if (badurls_it->second.list_name == list_name0 ||
+ badurls_it->second.list_name == list_name1) {
+ *prefix_hits = badurls_it->second.prefix_hits;
+ *full_hits = badurls_it->second.full_hits;
+ return true;
+ }
+
+ return false;
+ }
+
base::hash_map<std::string, Hits> badurls_;
};
@@ -193,7 +223,6 @@ class TestSBProtocolManagerFactory : public SBProtocolManagerFactory {
TestProtocolManager* pm_;
};
-
// Tests the safe browsing blocking page in a browser.
class SafeBrowsingServiceTest : public InProcessBrowserTest {
public:
@@ -236,6 +265,8 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
// This test will fill up the database using testing prefixes
// and urls.
command_line->AppendSwitch(switches::kSbDisableAutoUpdate);
+
+ command_line->AppendSwitch(switches::kSbEnableDownloadProtection);
}
virtual void SetUpInProcessBrowserTestFixture() {
@@ -253,7 +284,7 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
// full hash is hit in database's local cache.
std::vector<SBFullHashResult> empty_full_hits;
TestSafeBrowsingDatabase* db = db_factory_.GetDb();
- db->AddUrl(url, prefix_hits, empty_full_hits);
+ db->AddUrl(url, full_hash.list_name, prefix_hits, empty_full_hits);
TestProtocolManager* pm = pm_factory_.GetProtocolManager();
pm->SetGetFullHashResponse(url, full_hash);
@@ -273,11 +304,14 @@ class SafeBrowsingServiceTest : public InProcessBrowserTest {
};
const char kEmptyPage[] = "files/empty.html";
-const char kMalwarePage[] = "files/safe_browsing/malware.html";
+const char kMalwareFile[] = "files/downloads/dangerous/dangerous.exe";
const char kMalwareIframe[] = "files/safe_browsing/malware_iframe.html";
+const char kMalwarePage[] = "files/safe_browsing/malware.html";
+// This test goes through DownloadResourceHandler.
IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Malware) {
GURL url = test_server()->GetURL(kEmptyPage);
+
// After adding the url to safebrowsing database and getfullhash result,
// we should see the interstitial page.
SBFullHashResult malware_full_hash;
@@ -289,4 +323,71 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, Malware) {
EXPECT_TRUE(ShowingInterstitialPage());
}
+// This test uses SafeBrowsingService::Client to directly interact with
+// SafeBrowsingService.
+class TestSBClient
+ : public base::RefCountedThreadSafe<TestSBClient>,
+ public SafeBrowsingService::Client {
+ public:
+ TestSBClient() : result_(SafeBrowsingService::URL_SAFE),
+ safe_browsing_service_(g_browser_process->
+ resource_dispatcher_host()->
+ safe_browsing_service()) {
+ }
+
+ int GetResult() {
+ return result_;
+ }
+
+ void CheckUrl(const GURL& url) {
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ NewRunnableMethod(this, &TestSBClient::CheckUrlOnIOThread, url));
+ ui_test_utils::RunMessageLoop(); // Will stop in OnDownloadUrlCheckResult.
+ }
+
+ private:
+ void CheckUrlOnIOThread(const GURL& url) {
+ safe_browsing_service_->CheckDownloadUrl(url, this);
+ }
+
+ // Called when the result of checking a download URL is known.
+ void OnDownloadUrlCheckResult(const GURL& url,
+ SafeBrowsingService::UrlCheckResult result) {
+ result_ = result;
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+ NewRunnableMethod(this, &TestSBClient::DownloadUrlCheckDone));
+ }
+
+ void DownloadUrlCheckDone() {
+ MessageLoopForUI::current()->Quit();
+ }
+
+ SafeBrowsingService::UrlCheckResult result_;
+ SafeBrowsingService* safe_browsing_service_;
+
+ DISALLOW_COPY_AND_ASSIGN(TestSBClient);
+};
+
+IN_PROC_BROWSER_TEST_F(SafeBrowsingServiceTest, CheckDownloadUrl) {
+ GURL badbin_url = test_server()->GetURL(kMalwareFile);
+
+ scoped_refptr<TestSBClient> client(new TestSBClient);
+ client->CheckUrl(badbin_url);
+
+ // Since badbin_url is not in database, it is considered to be safe.
+ EXPECT_EQ(SafeBrowsingService::URL_SAFE, client->GetResult());
+
+ SBFullHashResult badbinurl_full_hash;
+ int chunk_id = 0;
+ GenerateFullhashResult(badbin_url, safe_browsing_util::kBinUrlList,
+ chunk_id, &badbinurl_full_hash);
+ SetupResponseForUrl(badbin_url, badbinurl_full_hash);
+
+ client->CheckUrl(badbin_url);
+
+ // Now, the badbin_url is not safe since it is added to download database.
+ EXPECT_EQ(SafeBrowsingService::BINARY_MALWARE, client->GetResult());
+}
+
} // namespace