Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(239)

Side by Side Diff: chrome/browser/safe_browsing/download_protection_service.cc

Issue 8536041: This CL integrates the new SafeBrowsing download service class (Closed) Base URL: http://git.chromium.org/git/chromium.git@trunk
Patch Set: fix unit-tests Created 9 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright (c) 2011 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/safe_browsing/download_protection_service.h" 5 #include "chrome/browser/safe_browsing/download_protection_service.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/format_macros.h"
8 #include "base/memory/scoped_ptr.h" 9 #include "base/memory/scoped_ptr.h"
9 #include "base/metrics/histogram.h" 10 #include "base/metrics/histogram.h"
10 #include "base/stl_util.h" 11 #include "base/stl_util.h"
11 #include "base/string_number_conversions.h" 12 #include "base/string_number_conversions.h"
12 #include "base/string_util.h" 13 #include "base/string_util.h"
13 #include "base/stringprintf.h" 14 #include "base/stringprintf.h"
14 #include "base/time.h" 15 #include "base/time.h"
15 #include "chrome/browser/safe_browsing/safe_browsing_service.h" 16 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
16 #include "chrome/browser/safe_browsing/signature_util.h" 17 #include "chrome/browser/safe_browsing/signature_util.h"
17 #include "chrome/common/net/http_return.h" 18 #include "chrome/common/net/http_return.h"
(...skipping 87 matching lines...) Expand 10 before | Expand all | Expand 10 after
105 // ALWAYS ADD NEW VALUES BEFORE THIS ONE. 106 // ALWAYS ADD NEW VALUES BEFORE THIS ONE.
106 DOWNLOAD_CHECKS_MAX 107 DOWNLOAD_CHECKS_MAX
107 }; 108 };
108 } // namespace 109 } // namespace
109 110
110 DownloadProtectionService::DownloadInfo::DownloadInfo() 111 DownloadProtectionService::DownloadInfo::DownloadInfo()
111 : total_bytes(0), user_initiated(false) {} 112 : total_bytes(0), user_initiated(false) {}
112 113
113 DownloadProtectionService::DownloadInfo::~DownloadInfo() {} 114 DownloadProtectionService::DownloadInfo::~DownloadInfo() {}
114 115
116 std::string DownloadProtectionService::DownloadInfo::DebugString() const {
117 std::string chain = "";
Brian Ryner 2011/11/14 22:10:39 Not necessary to initialize strings to empty.
noelutz 2011/11/15 00:14:10 Done.
118 for (size_t i = 0; i < download_url_chain.size(); ++i) {
119 chain += download_url_chain[i].spec();
120 if (i < download_url_chain.size() - 1) {
121 chain += " -> ";
122 }
123 }
124 return base::StringPrintf(
125 "DownloadInfo {download_url_chain:[%s], local_file:%s, "
Brian Ryner 2011/11/14 22:10:39 It might be useful to include the address of the o
noelutz 2011/11/15 00:14:10 Done.
126 "referrer_url:%s, sha256_hash:%s, total_bytes:%" PRId64 ", "
127 "user_initiated: %s}",
128 chain.c_str(),
129 local_file.value().c_str(),
130 referrer_url.spec().c_str(),
131 "TODO",
132 total_bytes,
133 user_initiated ? "true" : "false");
134 }
135
115 // static 136 // static
116 DownloadProtectionService::DownloadInfo 137 DownloadProtectionService::DownloadInfo
117 DownloadProtectionService::DownloadInfo::FromDownloadItem( 138 DownloadProtectionService::DownloadInfo::FromDownloadItem(
118 const DownloadItem& item) { 139 const DownloadItem& item) {
119 DownloadInfo download_info; 140 DownloadInfo download_info;
120 download_info.local_file = item.full_path(); 141 download_info.local_file = item.GetTargetFilePath();
Brian Ryner 2011/11/14 22:10:39 Can you explain what this change is for?
noelutz 2011/11/15 00:14:10 Woops. I don't think this will work. GetTargetFi
121 download_info.download_url_chain = item.url_chain(); 142 download_info.download_url_chain = item.url_chain();
122 download_info.referrer_url = item.referrer_url(); 143 download_info.referrer_url = item.referrer_url();
123 // TODO(bryner): Fill in the hash (we shouldn't compute it again) 144 // TODO(bryner): Fill in the hash (we shouldn't compute it again)
124 download_info.total_bytes = item.total_bytes(); 145 download_info.total_bytes = item.total_bytes();
125 // TODO(bryner): Populate user_initiated 146 // TODO(bryner): Populate user_initiated
126 return download_info; 147 return download_info;
127 } 148 }
128 149
129 // Parent SafeBrowsing::Client class used to lookup the bad binary 150 // Parent SafeBrowsing::Client class used to lookup the bad binary
130 // URL and digest list. There are two sub-classes (one for each list). 151 // URL and digest list. There are two sub-classes (one for each list).
(...skipping 23 matching lines...) Expand all
154 175
155 void CheckDone(SafeBrowsingService::UrlCheckResult sb_result) { 176 void CheckDone(SafeBrowsingService::UrlCheckResult sb_result) {
156 DownloadProtectionService::DownloadCheckResult result = 177 DownloadProtectionService::DownloadCheckResult result =
157 IsDangerous(sb_result) ? 178 IsDangerous(sb_result) ?
158 DownloadProtectionService::DANGEROUS : 179 DownloadProtectionService::DANGEROUS :
159 DownloadProtectionService::SAFE; 180 DownloadProtectionService::SAFE;
160 BrowserThread::PostTask(BrowserThread::UI, 181 BrowserThread::PostTask(BrowserThread::UI,
161 FROM_HERE, 182 FROM_HERE,
162 base::Bind(callback_, result)); 183 base::Bind(callback_, result));
163 UpdateDownloadCheckStats(total_type_); 184 UpdateDownloadCheckStats(total_type_);
164 if (IsDangerous(sb_result)) { 185 if (sb_result != SafeBrowsingService::SAFE) {
165 UpdateDownloadCheckStats(dangerous_type_); 186 UpdateDownloadCheckStats(dangerous_type_);
166 BrowserThread::PostTask( 187 BrowserThread::PostTask(
167 BrowserThread::UI, 188 BrowserThread::UI,
168 FROM_HERE, 189 FROM_HERE,
169 base::Bind(&DownloadSBClient::ReportMalware, 190 base::Bind(&DownloadSBClient::ReportMalware,
170 this, sb_result)); 191 this, sb_result));
171 } 192 }
172 } 193 }
173 194
174 void ReportMalware(SafeBrowsingService::UrlCheckResult result) { 195 void ReportMalware(SafeBrowsingService::UrlCheckResult result) {
(...skipping 87 matching lines...) Expand 10 before | Expand all | Expand 10 after
262 if (!sb_service_ || 283 if (!sb_service_ ||
263 sb_service_->CheckDownloadHash(info_.sha256_hash, this)) { 284 sb_service_->CheckDownloadHash(info_.sha256_hash, this)) {
264 CheckDone(SafeBrowsingService::SAFE); 285 CheckDone(SafeBrowsingService::SAFE);
265 } else { 286 } else {
266 AddRef(); // SafeBrowsingService takes a pointer not a scoped_refptr. 287 AddRef(); // SafeBrowsingService takes a pointer not a scoped_refptr.
267 } 288 }
268 } 289 }
269 290
270 virtual bool IsDangerous( 291 virtual bool IsDangerous(
271 SafeBrowsingService::UrlCheckResult result) const OVERRIDE { 292 SafeBrowsingService::UrlCheckResult result) const OVERRIDE {
272 return result == SafeBrowsingService::BINARY_MALWARE_HASH; 293 // We always return false here because we don't want to warn based on
294 // a match with the digest list. However, for UMA users, we want to
Brian Ryner 2011/11/14 22:10:39 I'm not sure I follow the comment -- what part of
noelutz 2011/11/15 00:14:10 Updated the comment. See CheckDone() above too.
295 // report the malware URL.
296 return false;
273 } 297 }
274 298
275 virtual void OnDownloadHashCheckResult( 299 virtual void OnDownloadHashCheckResult(
276 const std::string& hash, 300 const std::string& hash,
277 SafeBrowsingService::UrlCheckResult sb_result) OVERRIDE { 301 SafeBrowsingService::UrlCheckResult sb_result) OVERRIDE {
278 CheckDone(sb_result); 302 CheckDone(sb_result);
279 UMA_HISTOGRAM_TIMES("SB2.DownloadHashCheckDuration", 303 UMA_HISTOGRAM_TIMES("SB2.DownloadHashCheckDuration",
280 base::TimeTicks::Now() - start_time_); 304 base::TimeTicks::Now() - start_time_);
281 Release(); 305 Release();
282 } 306 }
(...skipping 19 matching lines...) Expand all
302 : info_(info), 326 : info_(info),
303 callback_(callback), 327 callback_(callback),
304 service_(service), 328 service_(service),
305 signature_util_(signature_util), 329 signature_util_(signature_util),
306 sb_service_(sb_service), 330 sb_service_(sb_service),
307 pingback_enabled_(service_->enabled()) { 331 pingback_enabled_(service_->enabled()) {
308 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); 332 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
309 } 333 }
310 334
311 void Start() { 335 void Start() {
336 VLOG(2) << "Starting SafeBrowsing download check for: "
337 << info_.DebugString();
312 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); 338 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
313 // TODO(noelutz): implement some cache to make sure we don't issue the same 339 // TODO(noelutz): implement some cache to make sure we don't issue the same
314 // request over and over again if a user downloads the same binary multiple 340 // request over and over again if a user downloads the same binary multiple
315 // times. 341 // times.
316 if (info_.download_url_chain.empty()) { 342 if (info_.download_url_chain.empty()) {
317 RecordImprovedProtectionStats(REASON_EMPTY_URL_CHAIN); 343 RecordImprovedProtectionStats(REASON_EMPTY_URL_CHAIN);
318 PostFinishTask(SAFE); 344 PostFinishTask(SAFE);
319 return; 345 return;
320 } 346 }
321 const GURL& final_url = info_.download_url_chain.back(); 347 const GURL& final_url = info_.download_url_chain.back();
322 if (!final_url.is_valid() || final_url.is_empty()) { 348 if (!final_url.is_valid() || final_url.is_empty()) {
323 RecordImprovedProtectionStats(REASON_INVALID_URL); 349 RecordImprovedProtectionStats(REASON_INVALID_URL);
324 PostFinishTask(SAFE); 350 PostFinishTask(SAFE);
325 return; 351 return;
326 } 352 }
327 RecordFileExtensionType(info_.local_file); 353 RecordFileExtensionType(info_.local_file);
328 354
329 if (!final_url.SchemeIs("http") || !IsBinaryFile(info_.local_file)) { 355 if (final_url.SchemeIs("https") || !IsBinaryFile(info_.local_file)) {
Brian Ryner 2011/11/14 22:10:39 So this might cause us to start checking non-HTTP
noelutz 2011/11/15 00:14:10 I think that's what we want.
330 RecordImprovedProtectionStats(!final_url.SchemeIs("http") ? 356 RecordImprovedProtectionStats(final_url.SchemeIs("https") ?
331 REASON_HTTPS_URL : REASON_NOT_BINARY_FILE); 357 REASON_HTTPS_URL : REASON_NOT_BINARY_FILE);
332 BrowserThread::PostTask( 358 BrowserThread::PostTask(
333 BrowserThread::IO, 359 BrowserThread::IO,
334 FROM_HERE, 360 FROM_HERE,
335 base::Bind(&CheckClientDownloadRequest::CheckDigestList, this)); 361 base::Bind(&CheckClientDownloadRequest::CheckDigestList, this));
336 return; 362 return;
337 } 363 }
338 364
339 // Compute features from the file contents. Note that we record histograms 365 // Compute features from the file contents. Note that we record histograms
340 // based on the result, so this runs regardless of whether the pingbacks 366 // based on the result, so this runs regardless of whether the pingbacks
(...skipping 278 matching lines...) Expand 10 before | Expand all | Expand 10 after
619 645
620 void DownloadProtectionService::RequestFinished( 646 void DownloadProtectionService::RequestFinished(
621 CheckClientDownloadRequest* request) { 647 CheckClientDownloadRequest* request) {
622 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); 648 DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
623 std::set<scoped_refptr<CheckClientDownloadRequest> >::iterator it = 649 std::set<scoped_refptr<CheckClientDownloadRequest> >::iterator it =
624 download_requests_.find(request); 650 download_requests_.find(request);
625 DCHECK(it != download_requests_.end()); 651 DCHECK(it != download_requests_.end());
626 download_requests_.erase(*it); 652 download_requests_.erase(*it);
627 } 653 }
628 } // namespace safe_browsing 654 } // namespace safe_browsing
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698