summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/download/chrome_download_manager_delegate.cc27
-rw-r--r--chrome/browser/download/chrome_download_manager_delegate.h6
-rw-r--r--chrome/browser/safe_browsing/client_side_detection_host_unittest.cc4
-rw-r--r--chrome/browser/safe_browsing/download_protection_service.cc412
-rw-r--r--chrome/browser/safe_browsing/download_protection_service.h84
-rw-r--r--chrome/browser/safe_browsing/download_protection_service_unittest.cc130
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc6
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_service.cc27
-rw-r--r--chrome/browser/safe_browsing/safe_browsing_service.h11
-rw-r--r--chrome/browser/safe_browsing/signature_util.h24
-rw-r--r--chrome/browser/safe_browsing/signature_util_posix.cc18
-rw-r--r--chrome/browser/safe_browsing/signature_util_win.cc31
-rw-r--r--chrome/browser/safe_browsing/signature_util_win_unittest.cc32
-rw-r--r--chrome/chrome_browser.gypi3
-rw-r--r--chrome/chrome_tests.gypi2
15 files changed, 581 insertions, 236 deletions
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index d524121..0d7bf68 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -156,8 +156,27 @@ bool ChromeDownloadManagerDelegate::ShouldOpenDownload(DownloadItem* item) {
}
bool ChromeDownloadManagerDelegate::ShouldCompleteDownload(DownloadItem* item) {
- if (!IsExtensionDownload(item))
+ if (!IsExtensionDownload(item)) {
+#if defined(ENABLE_SAFE_BROWSING)
+ // Begin the safe browsing download protection check.
+ SafeBrowsingService* sb_service =
+ g_browser_process->safe_browsing_service();
+ if (sb_service && sb_service->download_protection_service() &&
+ profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
+ using safe_browsing::DownloadProtectionService;
+ sb_service->download_protection_service()->CheckClientDownload(
+ DownloadProtectionService::DownloadInfo::FromDownloadItem(*item),
+ base::Bind(
+ &ChromeDownloadManagerDelegate::CheckClientDownloadDone,
+ this, item->id()));
+ // For now, we won't delay the download for this.
+ }
+#else
+ // Assume safe.
+#endif
+
return true;
+ }
scoped_refptr<CrxInstaller> crx_installer =
download_crx_util::OpenChromeExtension(profile_, *item);
@@ -227,6 +246,12 @@ void ChromeDownloadManagerDelegate::UpdatePathForItemInPersistentStore(
download_history_->UpdateDownloadPath(item, new_path);
}
+void ChromeDownloadManagerDelegate::CheckClientDownloadDone(
+ int32 download_id,
+ safe_browsing::DownloadProtectionService::DownloadCheckResult result) {
+ // TODO(bryner): notify the user based on this result
+}
+
void ChromeDownloadManagerDelegate::RemoveItemFromPersistentStore(
DownloadItem* item) {
download_history_->RemoveEntry(item);
diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h
index 0f87fa2..fe9a1cc 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.h
+++ b/chrome/browser/download/chrome_download_manager_delegate.h
@@ -11,6 +11,7 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/task.h"
+#include "chrome/browser/safe_browsing/download_protection_service.h"
#include "content/public/browser/download_manager_delegate.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
@@ -134,6 +135,11 @@ class ChromeDownloadManagerDelegate
// service.
void CheckDownloadHashDone(int32 download_id, bool is_dangerous_hash);
+ // Callback function after the DownloadProtectionService completes.
+ void CheckClientDownloadDone(
+ int32 download_id,
+ safe_browsing::DownloadProtectionService::DownloadCheckResult result);
+
Profile* profile_;
scoped_ptr<DownloadPrefs> download_prefs_;
scoped_ptr<DownloadHistory> download_history_;
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 60da4ea..90f8028 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
@@ -170,11 +170,13 @@ class ClientSideDetectionHostTest : public TabContentsWrapperTestHarness {
}
virtual void TearDown() {
- // Delete the host object on the UI thread.
+ // Delete the host object on the UI thread and release the
+ // SafeBrowsingService.
BrowserThread::PostTask(
BrowserThread::UI,
FROM_HERE,
new DeleteTask<ClientSideDetectionHost>(csd_host_.release()));
+ sb_service_ = NULL;
message_loop_.RunAllPending();
TabContentsWrapperTestHarness::TearDown();
io_thread_.reset();
diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc
index c1ce0f3..668290b 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -8,11 +8,15 @@
#include "base/memory/scoped_ptr.h"
#include "base/metrics/histogram.h"
#include "base/stl_util.h"
+#include "base/string_util.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/safe_browsing/signature_util.h"
#include "chrome/common/net/http_return.h"
#include "chrome/common/safe_browsing/csd.pb.h"
#include "content/browser/browser_thread.h"
+#include "content/browser/download/download_item.h"
#include "content/public/common/url_fetcher.h"
+#include "content/public/common/url_fetcher_delegate.h"
#include "net/base/load_flags.h"
#include "net/url_request/url_request_context_getter.h"
#include "net/url_request/url_request_status.h"
@@ -22,63 +26,110 @@ namespace safe_browsing {
const char DownloadProtectionService::kDownloadRequestUrl[] =
"https://sb-ssl.google.com/safebrowsing/clientreport/download";
+namespace {
+bool IsBinaryFile(const FilePath& file) {
+ return (file.MatchesExtension(FILE_PATH_LITERAL(".exe")) ||
+ file.MatchesExtension(FILE_PATH_LITERAL(".cab")) ||
+ file.MatchesExtension(FILE_PATH_LITERAL(".msi")));
+}
+} // namespace
+
DownloadProtectionService::DownloadInfo::DownloadInfo()
: total_bytes(0), user_initiated(false) {}
DownloadProtectionService::DownloadInfo::~DownloadInfo() {}
-DownloadProtectionService::DownloadProtectionService(
- SafeBrowsingService* sb_service,
- net::URLRequestContextGetter* request_context_getter)
- : sb_service_(sb_service),
- request_context_getter_(request_context_getter),
- enabled_(false) {}
-
-DownloadProtectionService::~DownloadProtectionService() {
- STLDeleteContainerPairFirstPointers(download_requests_.begin(),
- download_requests_.end());
- download_requests_.clear();
+// static
+DownloadProtectionService::DownloadInfo
+DownloadProtectionService::DownloadInfo::FromDownloadItem(
+ const DownloadItem& item) {
+ DownloadInfo download_info;
+ download_info.local_file = item.full_path();
+ download_info.download_url_chain = item.url_chain();
+ download_info.referrer_url = item.referrer_url();
+ // TODO(bryner): Fill in the hash (we shouldn't compute it again)
+ download_info.total_bytes = item.total_bytes();
+ // TODO(bryner): Populate user_initiated
+ return download_info;
}
-void DownloadProtectionService::SetEnabled(bool enabled) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- BrowserThread::PostTask(
- BrowserThread::IO,
- FROM_HERE,
- base::Bind(&DownloadProtectionService::SetEnabledOnIOThread,
- this, enabled));
-}
-
-void DownloadProtectionService::SetEnabledOnIOThread(bool enabled) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- if (enabled == enabled_) {
- return;
+class DownloadProtectionService::CheckClientDownloadRequest
+ : public base::RefCountedThreadSafe<
+ DownloadProtectionService::CheckClientDownloadRequest,
+ BrowserThread::DeleteOnUIThread>,
+ public content::URLFetcherDelegate {
+ public:
+ CheckClientDownloadRequest(const DownloadInfo& info,
+ const CheckDownloadCallback& callback,
+ DownloadProtectionService* service,
+ SafeBrowsingService* sb_service)
+ : info_(info),
+ callback_(callback),
+ service_(service),
+ sb_service_(sb_service),
+ pingback_enabled_(service_->enabled()) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
}
- enabled_ = enabled;
- if (!enabled_) {
- for (DownloadRequests::iterator it = download_requests_.begin();
- it != download_requests_.end(); ++it) {
- it->second.Run(SAFE);
+
+ void Start() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ // TODO(noelutz): implement some cache to make sure we don't issue the same
+ // request over and over again if a user downloads the same binary multiple
+ // times.
+ if (info_.download_url_chain.empty()) {
+ RecordStats(REASON_INVALID_URL);
+ PostFinishTask(SAFE);
+ return;
+ }
+ const GURL& final_url = info_.download_url_chain.back();
+ if (!final_url.is_valid() || final_url.is_empty() ||
+ !final_url.SchemeIs("http")) {
+ RecordStats(REASON_INVALID_URL);
+ PostFinishTask(SAFE);
+ return; // For now we only support HTTP download URLs.
}
- STLDeleteContainerPairFirstPointers(download_requests_.begin(),
- download_requests_.end());
- download_requests_.clear();
- }
-}
-void DownloadProtectionService::OnURLFetchComplete(
- const content::URLFetcher* source) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- scoped_ptr<const content::URLFetcher> s(source);
- if (download_requests_.find(source) != download_requests_.end()) {
- CheckDownloadCallback callback = download_requests_[source];
- download_requests_.erase(source);
- if (!enabled_) {
- // SafeBrowsing got disabled. We can't do anything. Note: the request
- // object will be deleted.
- RecordStats(REASON_SB_DISABLED);
+ if (!IsBinaryFile(info_.local_file)) {
+ RecordStats(REASON_NOT_BINARY_FILE);
+ PostFinishTask(SAFE);
return;
}
+
+ // Compute features from the file contents. Note that we record histograms
+ // based on the result, so this runs regardless of whether the pingbacks
+ // are enabled. Since we do blocking I/O, this happens on the file thread.
+ BrowserThread::PostTask(
+ BrowserThread::FILE,
+ FROM_HERE,
+ base::Bind(&CheckClientDownloadRequest::ExtractFileFeatures, this));
+ }
+
+ // Canceling a request will cause us to always report the result as SAFE.
+ // In addition, the DownloadProtectionService will not be notified when the
+ // request finishes, so it must drop its reference after calling Cancel.
+ void Cancel() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ service_ = NULL;
+ if (fetcher_.get()) {
+ // The DownloadProtectionService is going to release its reference, so we
+ // might be destroyed before the URLFetcher completes. Cancel the
+ // fetcher so it does not try to invoke OnURLFetchComplete.
+ FinishRequest(SAFE);
+ fetcher_.reset();
+ }
+ // Note: If there is no fetcher, then some callback is still holding a
+ // reference to this object. We'll eventually wind up in some method on
+ // the UI thread that will call FinishRequest() and run the callback.
+ }
+
+ // From the content::URLFetcherDelegate interface.
+ virtual void OnURLFetchComplete(const content::URLFetcher* source) OVERRIDE {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK_EQ(source, fetcher_.get());
+ VLOG(2) << "Received a response for URL: "
+ << info_.download_url_chain.back() << ": success="
+ << source->GetStatus().is_success() << " response_code="
+ << source->GetResponseCode();
DownloadCheckResultReason reason = REASON_MAX;
reason = REASON_SERVER_PING_FAILED;
if (source->GetStatus().is_success() &&
@@ -92,118 +143,207 @@ void DownloadProtectionService::OnURLFetchComplete(
reason = REASON_INVALID_RESPONSE_PROTO;
}
}
- BrowserThread::PostTask(
- BrowserThread::UI,
- FROM_HERE,
- base::Bind(&DownloadProtectionService::EndCheckClientDownload,
- this, SAFE, reason, callback));
- } else {
- NOTREACHED();
- }
-}
-bool DownloadProtectionService::CheckClientDownload(
- const DownloadInfo& info,
- const CheckDownloadCallback& callback) {
- // TODO(noelutz): implement some cache to make sure we don't issue the same
- // request over and over again if a user downloads the same binary multiple
- // times.
- if (info.download_url_chain.empty()) {
- RecordStats(REASON_INVALID_URL);
- return true;
- }
- const GURL& final_url = info.download_url_chain.back();
- if (!final_url.is_valid() || final_url.is_empty() ||
- !final_url.SchemeIs("http")) {
- RecordStats(REASON_INVALID_URL);
- return true; // For now we only support HTTP download URLs.
+ if (reason != REASON_MAX) {
+ RecordStats(reason);
+ }
+ // We don't need the fetcher anymore.
+ fetcher_.reset();
+ FinishRequest(SAFE);
}
- // TODO(noelutz): DownloadInfo should also contain the IP address of every
- // URL in the redirect chain. We also should check whether the download URL
- // is hosted on the internal network.
- BrowserThread::PostTask(
- BrowserThread::IO,
- FROM_HERE,
- base::Bind(&DownloadProtectionService::StartCheckClientDownload,
- this, info, callback));
- return false;
-}
-void DownloadProtectionService::StartCheckClientDownload(
- const DownloadInfo& info,
- const CheckDownloadCallback& callback) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- if (!enabled_ || !sb_service_.get()) {
- // This is a hard fail. We won't even call the callback in this case.
- RecordStats(REASON_SB_DISABLED);
- return;
+ private:
+ friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
+ friend class DeleteTask<CheckClientDownloadRequest>;
+
+ virtual ~CheckClientDownloadRequest() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
}
- DownloadCheckResultReason reason = REASON_MAX;
- for (size_t i = 0; i < info.download_url_chain.size(); ++i) {
- if (sb_service_->MatchDownloadWhitelistUrl(info.download_url_chain[i])) {
- reason = REASON_WHITELISTED_URL;
- break;
+
+ void ExtractFileFeatures() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+ bool is_signed;
+ if (safe_browsing::signature_util::IsSigned(info_.local_file)) {
+ VLOG(2) << "Downloaded a signed binary: " << info_.local_file.value();
+ is_signed = true;
+ } else {
+ VLOG(2) << "Downloaded an unsigned binary: " << info_.local_file.value();
+ is_signed = false;
}
+ UMA_HISTOGRAM_BOOLEAN("SBClientDownload.SignedBinaryDownload", is_signed);
+
+ // TODO(noelutz): DownloadInfo should also contain the IP address of every
+ // URL in the redirect chain. We also should check whether the download
+ // URL is hosted on the internal network.
+ BrowserThread::PostTask(
+ BrowserThread::IO,
+ FROM_HERE,
+ base::Bind(&CheckClientDownloadRequest::CheckWhitelists, this));
}
- if (sb_service_->MatchDownloadWhitelistUrl(info.referrer_url)) {
- reason = REASON_WHITELISTED_REFERRER;
- }
- // TODO(noelutz): check signature and CA against whitelist.
-
- ClientDownloadRequest request;
- request.set_url(info.download_url_chain.back().spec());
- request.mutable_digests()->set_sha256(info.sha256_hash);
- request.set_length(info.total_bytes);
- for (size_t i = 0; i < info.download_url_chain.size(); ++i) {
- ClientDownloadRequest::Resource* resource = request.add_resources();
- resource->set_url(info.download_url_chain[i].spec());
- if (i == info.download_url_chain.size() - 1) {
- // The last URL in the chain is the download URL.
- resource->set_type(ClientDownloadRequest::DOWNLOAD_URL);
- resource->set_referrer(info.referrer_url.spec());
+
+ void CheckWhitelists() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ DownloadCheckResultReason reason = REASON_MAX;
+ if (!pingback_enabled_ || !sb_service_.get()) {
+ reason = REASON_SB_DISABLED;
} else {
- resource->set_type(ClientDownloadRequest::DOWNLOAD_REDIRECT);
+ for (size_t i = 0; i < info_.download_url_chain.size(); ++i) {
+ const GURL& url = info_.download_url_chain[i];
+ if (url.is_valid() && sb_service_->MatchDownloadWhitelistUrl(url)) {
+ reason = REASON_WHITELISTED_URL;
+ break;
+ }
+ }
+ if (info_.referrer_url.is_valid() &&
+ sb_service_->MatchDownloadWhitelistUrl(info_.referrer_url)) {
+ reason = REASON_WHITELISTED_REFERRER;
+ }
+ }
+ if (reason != REASON_MAX) {
+ RecordStats(reason);
+ PostFinishTask(SAFE);
+ return;
}
- // TODO(noelutz): fill out the remote IP addresses.
+
+ // TODO(noelutz): check signature and CA against whitelist.
+
+ // The URLFetcher is owned by the UI thread, so post a message to
+ // start the pingback.
+ BrowserThread::PostTask(
+ BrowserThread::UI,
+ FROM_HERE,
+ base::Bind(&CheckClientDownloadRequest::SendRequest, this));
}
- request.set_user_initiated(info.user_initiated);
- std::string request_data;
- if (!request.SerializeToString(&request_data)) {
- reason = REASON_INVALID_REQUEST_PROTO;
+
+ void SendRequest() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+ // This is our last chance to check whether the request has been canceled
+ // before sending it.
+ if (!service_) {
+ FinishRequest(SAFE);
+ return;
+ }
+
+ ClientDownloadRequest request;
+ request.set_url(info_.download_url_chain.back().spec());
+ request.mutable_digests()->set_sha256(info_.sha256_hash);
+ request.set_length(info_.total_bytes);
+ for (size_t i = 0; i < info_.download_url_chain.size(); ++i) {
+ ClientDownloadRequest::Resource* resource = request.add_resources();
+ resource->set_url(info_.download_url_chain[i].spec());
+ if (i == info_.download_url_chain.size() - 1) {
+ // The last URL in the chain is the download URL.
+ resource->set_type(ClientDownloadRequest::DOWNLOAD_URL);
+ resource->set_referrer(info_.referrer_url.spec());
+ } else {
+ resource->set_type(ClientDownloadRequest::DOWNLOAD_REDIRECT);
+ }
+ // TODO(noelutz): fill out the remote IP addresses.
+ }
+ request.set_user_initiated(info_.user_initiated);
+ std::string request_data;
+ if (!request.SerializeToString(&request_data)) {
+ RecordStats(REASON_INVALID_REQUEST_PROTO);
+ FinishRequest(SAFE);
+ return;
+ }
+
+ VLOG(2) << "Sending a request for URL: "
+ << info_.download_url_chain.back();
+ fetcher_.reset(content::URLFetcher::Create(0 /* ID used for testing */,
+ GURL(kDownloadRequestUrl),
+ content::URLFetcher::POST,
+ this));
+ fetcher_->SetLoadFlags(net::LOAD_DISABLE_CACHE);
+ fetcher_->SetRequestContext(service_->request_context_getter_.get());
+ fetcher_->SetUploadData("application/octet-stream", request_data);
+ fetcher_->Start();
}
- if (reason != REASON_MAX) {
- // We stop here because the download is considered safe.
+ void PostFinishTask(DownloadCheckResult result) {
BrowserThread::PostTask(
BrowserThread::UI,
FROM_HERE,
- base::Bind(&DownloadProtectionService::EndCheckClientDownload,
- this, SAFE, reason, callback));
- return;
+ base::Bind(&CheckClientDownloadRequest::FinishRequest, this, result));
}
- content::URLFetcher* fetcher = content::URLFetcher::Create(
- 0 /* ID used for testing */, GURL(kDownloadRequestUrl),
- content::URLFetcher::POST, this);
- download_requests_[fetcher] = callback;
- fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE);
- fetcher->SetRequestContext(request_context_getter_.get());
- fetcher->SetUploadData("application/octet-stream", request_data);
- fetcher->Start();
+ void FinishRequest(DownloadCheckResult result) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ if (service_) {
+ callback_.Run(result);
+ service_->RequestFinished(this);
+ } else {
+ callback_.Run(SAFE);
+ }
+ }
+
+ void RecordStats(DownloadCheckResultReason reason) {
+ UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckDownloadStats",
+ reason,
+ REASON_MAX);
+ }
+
+ DownloadInfo info_;
+ CheckDownloadCallback callback_;
+ // Will be NULL if the request has been canceled.
+ DownloadProtectionService* service_;
+ scoped_refptr<SafeBrowsingService> sb_service_;
+ bool pingback_enabled_;
+ scoped_ptr<content::URLFetcher> fetcher_;
+
+ DISALLOW_COPY_AND_ASSIGN(CheckClientDownloadRequest);
+};
+
+DownloadProtectionService::DownloadProtectionService(
+ SafeBrowsingService* sb_service,
+ net::URLRequestContextGetter* request_context_getter)
+ : sb_service_(sb_service),
+ request_context_getter_(request_context_getter),
+ enabled_(false) {}
+
+DownloadProtectionService::~DownloadProtectionService() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ CancelPendingRequests();
+}
+
+void DownloadProtectionService::SetEnabled(bool enabled) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ if (enabled == enabled_) {
+ return;
+ }
+ enabled_ = enabled;
+ if (!enabled_) {
+ CancelPendingRequests();
+ }
}
-void DownloadProtectionService::EndCheckClientDownload(
- DownloadCheckResult result,
- DownloadCheckResultReason reason,
+void DownloadProtectionService::CheckClientDownload(
+ const DownloadProtectionService::DownloadInfo& info,
const CheckDownloadCallback& callback) {
+ scoped_refptr<CheckClientDownloadRequest> request(
+ new CheckClientDownloadRequest(info, callback, this, sb_service_));
+ download_requests_.insert(request);
+ request->Start();
+}
+
+void DownloadProtectionService::CancelPendingRequests() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- RecordStats(reason);
- callback.Run(result);
+ for (std::set<scoped_refptr<CheckClientDownloadRequest> >::iterator it =
+ download_requests_.begin();
+ it != download_requests_.end(); ++it) {
+ (*it)->Cancel();
+ }
+ download_requests_.clear();
}
-void DownloadProtectionService::RecordStats(DownloadCheckResultReason reason) {
- UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckDownloadStats",
- reason,
- REASON_MAX);
+void DownloadProtectionService::RequestFinished(
+ CheckClientDownloadRequest* request) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ std::set<scoped_refptr<CheckClientDownloadRequest> >::iterator it =
+ download_requests_.find(request);
+ DCHECK(it != download_requests_.end());
+ download_requests_.erase(*it);
}
+
} // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/download_protection_service.h b/chrome/browser/safe_browsing/download_protection_service.h
index 35345fc..ba186b9 100644
--- a/chrome/browser/safe_browsing/download_protection_service.h
+++ b/chrome/browser/safe_browsing/download_protection_service.h
@@ -9,36 +9,34 @@
#define CHROME_BROWSER_SAFE_BROWSING_DOWNLOAD_PROTECTION_SERVICE_H_
#pragma once
-#include <map>
+#include <set>
#include <string>
#include <vector>
#include "base/basictypes.h"
#include "base/callback.h"
+#include "base/file_path.h"
#include "base/gtest_prod_util.h"
#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/time.h"
-#include "content/public/common/url_fetcher_delegate.h"
#include "googleurl/src/gurl.h"
namespace net {
class URLRequestContextGetter;
class URLRequestStatus;
} // namespace net
+class DownloadItem;
class SafeBrowsingService;
namespace safe_browsing {
// This class provides an asynchronous API to check whether a particular
// client download is malicious or not.
-class DownloadProtectionService
- : public base::RefCountedThreadSafe<DownloadProtectionService>,
- public content::URLFetcherDelegate {
+class DownloadProtectionService {
public:
- // TODO(noelutz): we're missing some fields here: filename to get
- // the signature, server IPs, tab URL redirect chain, ...
+ // TODO(noelutz): we're missing some fields here: server IPs,
+ // tab URL redirect chain, ...
struct DownloadInfo {
+ FilePath local_file;
std::vector<GURL> download_url_chain;
GURL referrer_url;
std::string sha256_hash;
@@ -46,6 +44,9 @@ class DownloadProtectionService
bool user_initiated;
DownloadInfo();
~DownloadInfo();
+
+ // Creates a DownloadInfo from a DownloadItem object.
+ static DownloadInfo FromDownloadItem(const DownloadItem& item);
};
enum DownloadCheckResult {
@@ -59,32 +60,25 @@ class DownloadProtectionService
typedef base::Callback<void(DownloadCheckResult)> CheckDownloadCallback;
// Creates a download service. The service is initially disabled. You need
- // to call SetEnabled() to start it. We keep scoped references to both of
- // these objects.
+ // to call SetEnabled() to start it. |sb_service| owns this object; we
+ // keep a reference to |request_context_getter|.
DownloadProtectionService(
SafeBrowsingService* sb_service,
net::URLRequestContextGetter* request_context_getter);
- // From the content::URLFetcherDelegate interface.
- virtual void OnURLFetchComplete(const content::URLFetcher* source) OVERRIDE;
-
- // Checks whether the given client download is likely to be
- // malicious or not. If this method returns true it means the
- // download is safe or we're unable to tell whether it is safe or
- // not. In this case the callback will never be called. If this
- // method returns false we will asynchronously check whether the
- // download is safe and call the callback when we have the response.
- // This method should be called on the UI thread. The callback will
- // be called on the UI thread and will always be called asynchronously.
- virtual bool CheckClientDownload(const DownloadInfo& info,
+ virtual ~DownloadProtectionService();
+
+ // Checks whether the given client download is likely to be malicious or not.
+ // The result is delivered asynchronously via the given callback. This
+ // method must be called on the UI thread, and the callback will also be
+ // invoked on the UI thread.
+ virtual void CheckClientDownload(const DownloadInfo& info,
const CheckDownloadCallback& callback);
// Enables or disables the service. This is usually called by the
// SafeBrowsingService, which tracks whether any profile uses these services
- // at all. Disabling cancels any pending requests; existing requests will
- // have their callbacks called with "SAFE" results. Note: SetEnabled() is
- // asynchronous because this method is called on the UI thread but most
- // everything else happens on the IO thread.
+ // at all. Disabling causes any pending and future requests to have their
+ // callbacks called with "SAFE" results.
void SetEnabled(bool enabled);
bool enabled() const {
@@ -102,13 +96,12 @@ class DownloadProtectionService
REASON_INVALID_REQUEST_PROTO,
REASON_SERVER_PING_FAILED,
REASON_INVALID_RESPONSE_PROTO,
+ REASON_NOT_BINARY_FILE,
REASON_MAX // Always add new values before this one.
};
- virtual ~DownloadProtectionService();
-
private:
- friend class base::RefCountedThreadSafe<DownloadProtectionService>;
+ class CheckClientDownloadRequest; // Per-request state
friend class DownloadProtectionServiceTest;
FRIEND_TEST_ALL_PREFIXES(DownloadProtectionServiceTest,
CheckClientDownloadValidateRequest);
@@ -119,25 +112,20 @@ class DownloadProtectionService
static const char kDownloadRequestUrl[];
- // Same as above but this method is called on the IO thread after we have
- // done some basic checks to see whether the download is definitely not
- // safe.
- void StartCheckClientDownload(const DownloadInfo& info,
- const CheckDownloadCallback& callback);
-
- // This function must run on the UI thread and will invoke the callback
- // with the given result.
- void EndCheckClientDownload(DownloadCheckResult result,
- DownloadCheckResultReason reason,
- const CheckDownloadCallback& callback);
+ // Cancels all requests in |download_requests_|, and empties it, releasing
+ // the references to the requests.
+ void CancelPendingRequests();
- void RecordStats(DownloadCheckResultReason reason);
+ // Called by a CheckClientDownloadRequest instance when it finishes, to
+ // remove it from |download_requests_|.
+ void RequestFinished(CheckClientDownloadRequest* request);
- // SetEnabled(bool) calls this method on the IO thread.
- void SetEnabledOnIOThread(bool enableed);
+ static void FillDownloadInfo(const DownloadItem& item,
+ DownloadInfo* download_info);
- // This pointer may be NULL if SafeBrowsing is disabled.
- scoped_refptr<SafeBrowsingService> sb_service_;
+ // This pointer may be NULL if SafeBrowsing is disabled. The
+ // SafeBrowsingService owns us, so we don't need to hold a reference to it.
+ SafeBrowsingService* sb_service_;
// The context we use to issue network requests.
scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
@@ -145,9 +133,7 @@ class DownloadProtectionService
// Map of client download request to the corresponding callback that
// has to be invoked when the request is done. This map contains all
// pending server requests.
- typedef std::map<const content::URLFetcher*, CheckDownloadCallback>
- DownloadRequests;
- DownloadRequests download_requests_;
+ std::set<scoped_refptr<CheckClientDownloadRequest> > download_requests_;
// Keeps track of the state of the service.
bool enabled_;
diff --git a/chrome/browser/safe_browsing/download_protection_service_unittest.cc b/chrome/browser/safe_browsing/download_protection_service_unittest.cc
index 87a0378..ff2cc23 100644
--- a/chrome/browser/safe_browsing/download_protection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/download_protection_service_unittest.cc
@@ -9,12 +9,15 @@
#include "base/bind.h"
#include "base/callback.h"
+#include "base/file_path.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/message_loop.h"
#include "chrome/common/safe_browsing/csd.pb.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "content/browser/browser_thread.h"
+#include "content/browser/download/download_item.h"
+#include "content/public/common/url_fetcher_delegate.h"
#include "content/test/test_url_fetcher_factory.h"
#include "googleurl/src/gurl.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -41,19 +44,25 @@ class DownloadProtectionServiceTest : public testing::Test {
protected:
virtual void SetUp() {
ui_thread_.reset(new BrowserThread(BrowserThread::UI, &msg_loop_));
- io_thread_.reset(new BrowserThread(BrowserThread::IO, &msg_loop_));
+ // Start real threads for the IO and File threads so that the DCHECKs
+ // to test that we're on the correct thread work.
+ io_thread_.reset(new BrowserThread(BrowserThread::IO));
+ ASSERT_TRUE(io_thread_->Start());
+ file_thread_.reset(new BrowserThread(BrowserThread::FILE));
+ ASSERT_TRUE(file_thread_->Start());
sb_service_ = new MockSafeBrowsingService();
- download_service_ = new DownloadProtectionService(sb_service_.get(),
- NULL);
+ download_service_ = sb_service_->download_protection_service();
download_service_->SetEnabled(true);
msg_loop_.RunAllPending();
}
virtual void TearDown() {
- msg_loop_.RunAllPending();
- download_service_ = NULL;
+ // Flush all of the thread message loops to ensure that there are no
+ // tasks currently running.
+ FlushThreadMessageLoops();
sb_service_ = NULL;
io_thread_.reset();
+ file_thread_.reset();
ui_thread_.reset();
}
@@ -71,6 +80,45 @@ class DownloadProtectionServiceTest : public testing::Test {
return false;
}
+ // Flushes any pending tasks in the message loops of all threads.
+ void FlushThreadMessageLoops() {
+ FlushMessageLoop(BrowserThread::FILE);
+ FlushMessageLoop(BrowserThread::IO);
+ msg_loop_.RunAllPending();
+ }
+
+ private:
+ // Helper functions for FlushThreadMessageLoops.
+ void RunAllPendingAndQuitUI() {
+ MessageLoop::current()->RunAllPending();
+ BrowserThread::PostTask(
+ BrowserThread::UI,
+ FROM_HERE,
+ base::Bind(&DownloadProtectionServiceTest::QuitMessageLoop,
+ base::Unretained(this)));
+ }
+
+ void QuitMessageLoop() {
+ MessageLoop::current()->Quit();
+ }
+
+ void PostRunMessageLoopTask(BrowserThread::ID thread) {
+ BrowserThread::PostTask(
+ thread,
+ FROM_HERE,
+ base::Bind(&DownloadProtectionServiceTest::RunAllPendingAndQuitUI,
+ base::Unretained(this)));
+ }
+
+ void FlushMessageLoop(BrowserThread::ID thread) {
+ BrowserThread::PostTask(
+ BrowserThread::UI,
+ FROM_HERE,
+ base::Bind(&DownloadProtectionServiceTest::PostRunMessageLoopTask,
+ base::Unretained(this), thread));
+ msg_loop_.Run();
+ }
+
public:
void CheckDoneCallback(
DownloadProtectionService::DownloadCheckResult result) {
@@ -78,36 +126,51 @@ class DownloadProtectionServiceTest : public testing::Test {
msg_loop_.Quit();
}
+ void SendURLFetchComplete(TestURLFetcher* fetcher) {
+ fetcher->delegate()->OnURLFetchComplete(fetcher);
+ }
+
protected:
scoped_refptr<MockSafeBrowsingService> sb_service_;
- scoped_refptr<DownloadProtectionService> download_service_;
+ DownloadProtectionService* download_service_;
MessageLoop msg_loop_;
DownloadProtectionService::DownloadCheckResult result_;
scoped_ptr<BrowserThread> io_thread_;
+ scoped_ptr<BrowserThread> file_thread_;
scoped_ptr<BrowserThread> ui_thread_;
};
TEST_F(DownloadProtectionServiceTest, CheckClientDownloadInvalidUrl) {
DownloadProtectionService::DownloadInfo info;
- EXPECT_TRUE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
+ base::Unretained(this)));
+ msg_loop_.Run();
+ EXPECT_EQ(DownloadProtectionService::SAFE, result_);
+
// Only http is supported for now.
+ info.local_file = FilePath(FILE_PATH_LITERAL("a.exe"));
info.download_url_chain.push_back(GURL("https://www.google.com/"));
- EXPECT_TRUE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
+ base::Unretained(this)));
+ msg_loop_.Run();
+ EXPECT_EQ(DownloadProtectionService::SAFE, result_);
+
info.download_url_chain[0] = GURL("ftp://www.google.com/");
- EXPECT_TRUE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
+ base::Unretained(this)));
+ msg_loop_.Run();
+ EXPECT_EQ(DownloadProtectionService::SAFE, result_);
}
TEST_F(DownloadProtectionServiceTest, CheckClientDownloadWhitelistedUrl) {
DownloadProtectionService::DownloadInfo info;
+ info.local_file = FilePath(FILE_PATH_LITERAL("a.exe"));
info.download_url_chain.push_back(GURL("http://www.evil.com/bla.exe"));
info.download_url_chain.push_back(GURL("http://www.google.com/a.exe"));
info.referrer_url = GURL("http://www.google.com/");
@@ -118,20 +181,20 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadWhitelistedUrl) {
MatchDownloadWhitelistUrl(GURL("http://www.google.com/a.exe")))
.WillRepeatedly(Return(true));
- EXPECT_FALSE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
+ base::Unretained(this)));
msg_loop_.Run();
EXPECT_EQ(DownloadProtectionService::SAFE, result_);
// Check that the referrer is matched against the whitelist.
info.download_url_chain.pop_back();
info.referrer_url = GURL("http://www.google.com/a.exe");
- EXPECT_FALSE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
+ base::Unretained(this)));
msg_loop_.Run();
EXPECT_EQ(DownloadProtectionService::SAFE, result_);
}
@@ -146,12 +209,13 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadFetchFailed) {
.WillRepeatedly(Return(false));
DownloadProtectionService::DownloadInfo info;
+ info.local_file = FilePath(FILE_PATH_LITERAL("a.exe"));
info.download_url_chain.push_back(GURL("http://www.evil.com/a.exe"));
info.referrer_url = GURL("http://www.google.com/");
- EXPECT_FALSE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
+ base::Unretained(this)));
msg_loop_.Run();
EXPECT_EQ(DownloadProtectionService::SAFE, result_);
}
@@ -166,12 +230,13 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
.WillRepeatedly(Return(false));
DownloadProtectionService::DownloadInfo info;
+ info.local_file = FilePath(FILE_PATH_LITERAL("a.exe"));
info.download_url_chain.push_back(GURL("http://www.evil.com/a.exe"));
info.referrer_url = GURL("http://www.google.com/");
- EXPECT_FALSE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
+ base::Unretained(this)));
msg_loop_.Run();
EXPECT_EQ(DownloadProtectionService::SAFE, result_);
@@ -179,10 +244,10 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
factory.SetFakeResponse(
DownloadProtectionService::kDownloadRequestUrl, "bla", true);
- EXPECT_FALSE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
+ base::Unretained(this)));
msg_loop_.Run();
EXPECT_EQ(DownloadProtectionService::SAFE, result_);
}
@@ -191,6 +256,7 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadValidateRequest) {
TestURLFetcherFactory factory;
DownloadProtectionService::DownloadInfo info;
+ info.local_file = FilePath(FILE_PATH_LITERAL("bla.exe"));
info.download_url_chain.push_back(GURL("http://www.google.com/"));
info.download_url_chain.push_back(GURL("http://www.google.com/bla.exe"));
info.referrer_url = GURL("http://www.google.com/");
@@ -200,16 +266,17 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadValidateRequest) {
EXPECT_CALL(*sb_service_, MatchDownloadWhitelistUrl(_))
.WillRepeatedly(Return(false));
- EXPECT_FALSE(download_service_->CheckClientDownload(
+ download_service_->CheckClientDownload(
info,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
- base::Unretained(this))));
- msg_loop_.RunAllPending(); // Wait until StartCheckClientDownload is called.
+ base::Unretained(this)));
+ // Run the message loop(s) until SendRequest is called.
+ FlushThreadMessageLoops();
- ASSERT_TRUE(factory.GetFetcherByID(0));
+ TestURLFetcher* fetcher = factory.GetFetcherByID(0);
+ ASSERT_TRUE(fetcher);
ClientDownloadRequest request;
- EXPECT_TRUE(request.ParseFromString(
- factory.GetFetcherByID(0)->upload_data()));
+ EXPECT_TRUE(request.ParseFromString(fetcher->upload_data()));
EXPECT_EQ("http://www.google.com/bla.exe", request.url());
EXPECT_EQ(info.sha256_hash, request.digests().sha256());
EXPECT_EQ(info.total_bytes, request.length());
@@ -222,5 +289,12 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadValidateRequest) {
ClientDownloadRequest::DOWNLOAD_URL,
"http://www.google.com/bla.exe",
info.referrer_url.spec()));
+
+ // Simulate the request finishing.
+ MessageLoop::current()->PostTask(
+ FROM_HERE,
+ base::Bind(&DownloadProtectionServiceTest::SendURLFetchComplete,
+ base::Unretained(this), fetcher));
+ msg_loop_.Run();
}
} // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
index f1473f4..4367305 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
@@ -90,6 +90,12 @@ class SafeBrowsingBlockingPageTest : public ChromeRenderViewHostTestHarness,
ResetUserResponse();
}
+ virtual void TearDown() {
+ // Release the SafeBrowsingService before the BrowserThreads are destroyed.
+ service_ = NULL;
+ ChromeRenderViewHostTestHarness::TearDown();
+ }
+
// SafeBrowsingService::Client implementation.
virtual void OnUrlCheckResult(const GURL& url,
SafeBrowsingService::UrlCheckResult result) {
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 914a9fe..8987512 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -30,7 +30,6 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
-#include "content/browser/browser_thread.h"
#include "content/browser/tab_contents/tab_contents.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
@@ -175,12 +174,9 @@ SafeBrowsingService::SafeBrowsingService()
safe_browsing::ClientSideDetectionService::Create(
g_browser_process->system_request_context()));
}
- if (CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kEnableImprovedDownloadProtection)) {
- download_service_ = new safe_browsing::DownloadProtectionService(
- this,
- g_browser_process->system_request_context());
- }
+ download_service_.reset(new safe_browsing::DownloadProtectionService(
+ this,
+ g_browser_process->system_request_context()));
#endif
}
@@ -216,20 +212,12 @@ void SafeBrowsingService::Initialize() {
}
void SafeBrowsingService::ShutDown() {
- if (download_service_.get()) {
- // Disabling the download service first will ensure that it is
- // disabled before the SafeBrowsingService object becomes invalid. The
- // download service might stay around for a bit since it's
- // ref-counted but it won't do any harm because it will be
- // disabled.
- download_service_->SetEnabled(false);
- download_service_ = NULL;
- }
Stop();
// The IO thread is going away, so make sure the ClientSideDetectionService
// dtor executes now since it may call the dtor of URLFetcher which relies
// on it.
csd_service_.reset();
+ download_service_.reset();
}
bool SafeBrowsingService::CanCheckUrl(const GURL& url) const {
@@ -1368,6 +1356,9 @@ void SafeBrowsingService::RefreshState() {
if (csd_service_.get())
csd_service_->SetEnabledAndRefreshState(enable);
- if (download_service_.get())
- download_service_->SetEnabled(enable);
+ if (download_service_.get()) {
+ download_service_->SetEnabled(
+ enable && CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kEnableImprovedDownloadProtection));
+ }
}
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index ccf3ab0..43a1c36 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -23,6 +23,7 @@
#include "base/task.h"
#include "base/time.h"
#include "chrome/browser/safe_browsing/safe_browsing_util.h"
+#include "content/browser/browser_thread.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "googleurl/src/gurl.h"
@@ -49,7 +50,8 @@ class DownloadProtectionService;
// Construction needs to happen on the main thread.
class SafeBrowsingService
- : public base::RefCountedThreadSafe<SafeBrowsingService>,
+ : public base::RefCountedThreadSafe<SafeBrowsingService,
+ BrowserThread::DeleteOnUIThread>,
public content::NotificationObserver {
public:
class Client;
@@ -267,6 +269,8 @@ class SafeBrowsingService
return csd_service_.get();
}
+ // The DownloadProtectionService is not valid after the SafeBrowsingService
+ // is destroyed.
safe_browsing::DownloadProtectionService*
download_protection_service() const {
return download_service_.get();
@@ -325,7 +329,8 @@ class SafeBrowsingService
base::TimeTicks start; // When check was queued.
};
- friend class base::RefCountedThreadSafe<SafeBrowsingService>;
+ friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
+ friend class DeleteTask<SafeBrowsingService>;
friend class SafeBrowsingServiceTest;
// Called to initialize objects that are used on the io_thread.
@@ -556,7 +561,7 @@ class SafeBrowsingService
// The DownloadProtectionService is managed by the SafeBrowsingService,
// since its running state and lifecycle depends on SafeBrowsingService's.
- scoped_refptr<safe_browsing::DownloadProtectionService> download_service_;
+ scoped_ptr<safe_browsing::DownloadProtectionService> download_service_;
DISALLOW_COPY_AND_ASSIGN(SafeBrowsingService);
};
diff --git a/chrome/browser/safe_browsing/signature_util.h b/chrome/browser/safe_browsing/signature_util.h
new file mode 100644
index 0000000..76c47ea
--- /dev/null
+++ b/chrome/browser/safe_browsing/signature_util.h
@@ -0,0 +1,24 @@
+// Copyright (c) 2011 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.
+//
+// Utility functions to check executable signatures for malicious binary
+// detection. Each platform has its own implementation of this class.
+
+#ifndef CHROME_BROWSER_SAFE_BROWSING_SIGNATURE_UTIL_H_
+#define CHROME_BROWSER_SAFE_BROWSING_SIGNATURE_UTIL_H_
+#pragma once
+
+class FilePath;
+
+namespace safe_browsing {
+namespace signature_util {
+
+// Returns true if the given file path contains a signature. No checks are
+// performed as to the validity of the signature.
+bool IsSigned(const FilePath& file_path);
+
+} // namespace signature_util
+} // namespace safe_browsing
+
+#endif // CHROME_BROWSER_SAFE_BROWSING_SIGNATURE_UTIL_H_
diff --git a/chrome/browser/safe_browsing/signature_util_posix.cc b/chrome/browser/safe_browsing/signature_util_posix.cc
new file mode 100644
index 0000000..5e99ca3
--- /dev/null
+++ b/chrome/browser/safe_browsing/signature_util_posix.cc
@@ -0,0 +1,18 @@
+// Copyright (c) 2011 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.
+//
+// This is a stub for the code signing utilities on Mac and Linux.
+// It should eventually be replaced with a real implementation.
+
+#include "chrome/browser/safe_browsing/signature_util.h"
+
+namespace safe_browsing {
+namespace signature_util {
+
+bool IsSigned(const FilePath& file_path) {
+ return false;
+}
+
+} // namespace signature_util
+} // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/signature_util_win.cc b/chrome/browser/safe_browsing/signature_util_win.cc
new file mode 100644
index 0000000..b7ae875
--- /dev/null
+++ b/chrome/browser/safe_browsing/signature_util_win.cc
@@ -0,0 +1,31 @@
+// Copyright (c) 2011 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 "chrome/browser/safe_browsing/signature_util.h"
+
+#include <windows.h>
+#include <wincrypt.h>
+#include "base/file_path.h"
+
+namespace safe_browsing {
+namespace signature_util {
+
+bool IsSigned(const FilePath& file_path) {
+ // Get message handle and store handle from the signed file.
+ BOOL result = CryptQueryObject(CERT_QUERY_OBJECT_FILE,
+ file_path.value().c_str(),
+ CERT_QUERY_CONTENT_FLAG_PKCS7_SIGNED_EMBED,
+ CERT_QUERY_FORMAT_FLAG_BINARY,
+ 0, // flags
+ NULL, // encoding
+ NULL, // content type
+ NULL, // format type
+ NULL, // HCERTSTORE
+ NULL, // HCRYPTMSG
+ NULL); // context
+ return result == TRUE;
+}
+
+} // namespace signature_util
+} // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/signature_util_win_unittest.cc b/chrome/browser/safe_browsing/signature_util_win_unittest.cc
new file mode 100644
index 0000000..b3131fc
--- /dev/null
+++ b/chrome/browser/safe_browsing/signature_util_win_unittest.cc
@@ -0,0 +1,32 @@
+// Copyright (c) 2011 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 "chrome/browser/safe_browsing/signature_util.h"
+
+#include "base/base_paths.h"
+#include "base/file_path.h"
+#include "base/path_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace safe_browsing {
+
+TEST(SignatureUtilWinTest, IsSigned) {
+ FilePath source_path;
+ ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &source_path));
+
+ FilePath testdata_path = source_path
+ .AppendASCII("chrome")
+ .AppendASCII("test")
+ .AppendASCII("data")
+ .AppendASCII("safe_browsing")
+ .AppendASCII("download_protection");
+
+ EXPECT_TRUE(signature_util::IsSigned(testdata_path.Append(L"signed.exe")));
+ EXPECT_FALSE(signature_util::IsSigned(
+ testdata_path.Append(L"unsigned.exe")));
+ EXPECT_FALSE(signature_util::IsSigned(
+ testdata_path.Append(L"doesnotexist.exe")));
+}
+
+} // namespace safe_browsing
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 313f018..7f8423b 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2055,6 +2055,9 @@
'browser/safe_browsing/safe_browsing_store_file.h',
'browser/safe_browsing/safe_browsing_util.cc',
'browser/safe_browsing/safe_browsing_util.h',
+ 'browser/safe_browsing/signature_util_posix.cc',
+ 'browser/safe_browsing/signature_util_win.cc',
+ 'browser/safe_browsing/signature_util.h',
'browser/screensaver_window_finder_gtk.cc',
'browser/screensaver_window_finder_gtk.h',
'browser/search_engines/search_engine_type.h',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index cc30bd3..96e09bc 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1508,6 +1508,7 @@
'browser/safe_browsing/safe_browsing_store_unittest.cc',
'browser/safe_browsing/safe_browsing_store_unittest_helper.cc',
'browser/safe_browsing/safe_browsing_util_unittest.cc',
+ 'browser/safe_browsing/signature_util_win_unittest.cc',
'browser/search_engines/search_host_to_urls_map_unittest.cc',
'browser/search_engines/search_provider_install_data_unittest.cc',
'browser/search_engines/template_url_fetcher_unittest.cc',
@@ -1989,6 +1990,7 @@
'sources/': [
['exclude', '^browser/password_manager/native_backend_gnome_x_unittest.cc'],
['exclude', '^browser/password_manager/native_backend_kwallet_x_unittest.cc'],
+ ['exclude', '^browser/safe_browsing/download_protection_service_unittest.cc' ],
['exclude', '^../content/browser/geolocation/wifi_data_provider_linux_unittest.cc'],
# TODO(thestig) Enable PrintPreviewUI tests on CrOS when
# print preview is enabled on CrOS.