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

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

Issue 2146703002: Sample 1% url whitelisted PPAPI downloads to ping safe browsing server (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: address thestig@'s comments Created 4 years, 5 months 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
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 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 <stddef.h> 7 #include <stddef.h>
8 8
9 #include <memory> 9 #include <memory>
10 10
(...skipping 55 matching lines...) Expand 10 before | Expand all | Expand 10 after
66 #include "net/url_request/url_fetcher_delegate.h" 66 #include "net/url_request/url_fetcher_delegate.h"
67 #include "net/url_request/url_request_status.h" 67 #include "net/url_request/url_request_status.h"
68 68
69 #if defined(OS_MACOSX) 69 #if defined(OS_MACOSX)
70 #include "chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.h" 70 #include "chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.h"
71 #endif 71 #endif
72 72
73 using content::BrowserThread; 73 using content::BrowserThread;
74 74
75 namespace { 75 namespace {
76 static const int64_t kDownloadRequestTimeoutMs = 7000; 76
77 const int64_t kDownloadRequestTimeoutMs = 7000;
77 // We sample 1% of whitelisted downloads to still send out download pings. 78 // We sample 1% of whitelisted downloads to still send out download pings.
78 static const double kWhitelistDownloadSampleRate = 0.01; 79 const double kWhitelistDownloadSampleRate = 0.01;
80
81 enum WhitelistType {
82 NO_WHITELIST_MATCH,
83 URL_WHITELIST,
84 SIGNATURE_WHITELIST,
85 WHITELIST_TYPE_MAX
86 };
87
88 void RecordCountOfWhitelistedDownload(WhitelistType type) {
89 UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckWhitelistResult", type,
90 WHITELIST_TYPE_MAX);
91 }
92
79 } // namespace 93 } // namespace
80 94
81 namespace safe_browsing { 95 namespace safe_browsing {
82 96
83 const char DownloadProtectionService::kDownloadRequestUrl[] = 97 const char DownloadProtectionService::kDownloadRequestUrl[] =
84 "https://sb-ssl.google.com/safebrowsing/clientreport/download"; 98 "https://sb-ssl.google.com/safebrowsing/clientreport/download";
85 99
86 const void* const DownloadProtectionService::kDownloadPingTokenKey 100 const void* const DownloadProtectionService::kDownloadPingTokenKey
87 = &kDownloadPingTokenKey; 101 = &kDownloadPingTokenKey;
88 102
(...skipping 664 matching lines...) Expand 10 before | Expand all | Expand 10 after
753 } else { 767 } else {
754 PostFinishTask(SAFE, REASON_ARCHIVE_WITHOUT_BINARIES); 768 PostFinishTask(SAFE, REASON_ARCHIVE_WITHOUT_BINARIES);
755 return; 769 return;
756 } 770 }
757 } 771 }
758 772
759 OnFileFeatureExtractionDone(); 773 OnFileFeatureExtractionDone();
760 } 774 }
761 #endif // defined(OS_MACOSX) 775 #endif // defined(OS_MACOSX)
762 776
763 enum WhitelistType { 777 bool ShouldSampleWhitelistedDownload() {
764 NO_WHITELIST_MATCH,
765 URL_WHITELIST,
766 SIGNATURE_WHITELIST,
767 WHITELIST_TYPE_MAX
768 };
769
770 static void RecordCountOfWhitelistedDownload(WhitelistType type) {
771 UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckWhitelistResult",
772 type,
773 WHITELIST_TYPE_MAX);
774 }
775
776 virtual bool ShouldSampleWhitelistedDownload() {
777 // We currently sample 1% whitelisted downloads from users who opted 778 // We currently sample 1% whitelisted downloads from users who opted
778 // in extended reporting and are not in incognito mode. 779 // in extended reporting and are not in incognito mode.
779 return service_ && is_extended_reporting_ && !is_incognito_ && 780 return service_ && is_extended_reporting_ && !is_incognito_ &&
780 base::RandDouble() < service_->whitelist_sample_rate(); 781 base::RandDouble() < service_->whitelist_sample_rate();
781 } 782 }
782 783
783 void CheckWhitelists() { 784 void CheckWhitelists() {
784 DCHECK_CURRENTLY_ON(BrowserThread::IO); 785 DCHECK_CURRENTLY_ON(BrowserThread::IO);
785 786
786 if (!database_manager_.get()) { 787 if (!database_manager_.get()) {
(...skipping 406 matching lines...) Expand 10 before | Expand all | Expand 10 after
1193 REQUEST_MALFORMED, 1194 REQUEST_MALFORMED,
1194 FETCH_FAILED, 1195 FETCH_FAILED,
1195 RESPONSE_MALFORMED, 1196 RESPONSE_MALFORMED,
1196 SUCCEEDED 1197 SUCCEEDED
1197 }; 1198 };
1198 1199
1199 PPAPIDownloadRequest( 1200 PPAPIDownloadRequest(
1200 const GURL& requestor_url, 1201 const GURL& requestor_url,
1201 const base::FilePath& default_file_path, 1202 const base::FilePath& default_file_path,
1202 const std::vector<base::FilePath::StringType>& alternate_extensions, 1203 const std::vector<base::FilePath::StringType>& alternate_extensions,
1204 Profile* profile,
1203 const CheckDownloadCallback& callback, 1205 const CheckDownloadCallback& callback,
1204 DownloadProtectionService* service, 1206 DownloadProtectionService* service,
1205 scoped_refptr<SafeBrowsingDatabaseManager> database_manager) 1207 scoped_refptr<SafeBrowsingDatabaseManager> database_manager)
1206 : requestor_url_(requestor_url), 1208 : requestor_url_(requestor_url),
1207 default_file_path_(default_file_path), 1209 default_file_path_(default_file_path),
1208 alternate_extensions_(alternate_extensions), 1210 alternate_extensions_(alternate_extensions),
1209 callback_(callback), 1211 callback_(callback),
1210 service_(service), 1212 service_(service),
1211 database_manager_(database_manager), 1213 database_manager_(database_manager),
1212 start_time_(base::TimeTicks::Now()), 1214 start_time_(base::TimeTicks::Now()),
1213 supported_path_( 1215 supported_path_(
1214 GetSupportedFilePath(default_file_path, alternate_extensions)), 1216 GetSupportedFilePath(default_file_path, alternate_extensions)),
1215 weakptr_factory_(this) {} 1217 sample_url_whitelist_(false),
1218 weakptr_factory_(this) {
1219 is_extended_reporting_ = profile &&
asanka 2016/07/15 14:59:52 Minor nit: Just noting that Profile had better be
Jialiu Lin 2016/07/15 22:10:56 Done.
1220 profile->GetPrefs()->GetBoolean(
1221 prefs::kSafeBrowsingExtendedReportingEnabled);
1222 is_incognito_ = profile && profile->IsOffTheRecord();
1223 }
1216 1224
1217 ~PPAPIDownloadRequest() override { 1225 ~PPAPIDownloadRequest() override {
1218 if (fetcher_ && !callback_.is_null()) 1226 if (fetcher_ && !callback_.is_null())
1219 Finish(RequestOutcome::REQUEST_DESTROYED, UNKNOWN); 1227 Finish(RequestOutcome::REQUEST_DESTROYED, UNKNOWN);
1220 } 1228 }
1221 1229
1222 // Start the process of checking the download request. The callback passed as 1230 // Start the process of checking the download request. The callback passed as
1223 // the |callback| parameter to the constructor will be invoked with the result 1231 // the |callback| parameter to the constructor will be invoked with the result
1224 // of the check at some point in the future. 1232 // of the check at some point in the future.
1225 // 1233 //
(...skipping 29 matching lines...) Expand all
1255 service_->download_request_timeout_ms())); 1263 service_->download_request_timeout_ms()));
1256 1264
1257 BrowserThread::PostTask( 1265 BrowserThread::PostTask(
1258 BrowserThread::IO, FROM_HERE, 1266 BrowserThread::IO, FROM_HERE,
1259 base::Bind(&PPAPIDownloadRequest::CheckWhitelistsOnIOThread, 1267 base::Bind(&PPAPIDownloadRequest::CheckWhitelistsOnIOThread,
1260 requestor_url_, database_manager_, 1268 requestor_url_, database_manager_,
1261 weakptr_factory_.GetWeakPtr())); 1269 weakptr_factory_.GetWeakPtr()));
1262 } 1270 }
1263 1271
1264 private: 1272 private:
1273 bool ShouldSampleWhitelistedDownload() {
1274 // We currently sample 1% whitelisted downloads from users who opted
1275 // in extended reporting and are not in incognito mode.
1276 return service_ && !is_incognito_ && is_extended_reporting_ &&
1277 base::RandDouble() < service_->whitelist_sample_rate();
1278 }
1279
1265 // Whitelist checking needs to the done on the IO thread. 1280 // Whitelist checking needs to the done on the IO thread.
1266 static void CheckWhitelistsOnIOThread( 1281 static void CheckWhitelistsOnIOThread(
1267 const GURL& requestor_url, 1282 const GURL& requestor_url,
1268 scoped_refptr<SafeBrowsingDatabaseManager> database_manager, 1283 scoped_refptr<SafeBrowsingDatabaseManager> database_manager,
1269 base::WeakPtr<PPAPIDownloadRequest> download_request) { 1284 base::WeakPtr<PPAPIDownloadRequest> download_request) {
1270 DCHECK_CURRENTLY_ON(BrowserThread::IO); 1285 DCHECK_CURRENTLY_ON(BrowserThread::IO);
1271 DVLOG(2) << " checking whitelists for requestor URL:" << requestor_url; 1286 DVLOG(2) << " checking whitelists for requestor URL:" << requestor_url;
1272 1287
1273 bool url_was_whitelisted = 1288 bool url_was_whitelisted =
1274 requestor_url.is_valid() && database_manager && 1289 requestor_url.is_valid() && database_manager &&
1275 database_manager->MatchDownloadWhitelistUrl(requestor_url); 1290 database_manager->MatchDownloadWhitelistUrl(requestor_url);
1276 BrowserThread::PostTask( 1291 BrowserThread::PostTask(
1277 BrowserThread::UI, FROM_HERE, 1292 BrowserThread::UI, FROM_HERE,
1278 base::Bind(&PPAPIDownloadRequest::WhitelistCheckComplete, 1293 base::Bind(&PPAPIDownloadRequest::WhitelistCheckComplete,
1279 download_request, url_was_whitelisted)); 1294 download_request, url_was_whitelisted));
1280 } 1295 }
1281 1296
1282 void WhitelistCheckComplete(bool was_on_whitelist) { 1297 void WhitelistCheckComplete(bool was_on_whitelist) {
1283 DVLOG(2) << __FUNCTION__ << " was_on_whitelist:" << was_on_whitelist; 1298 DVLOG(2) << __FUNCTION__ << " was_on_whitelist:" << was_on_whitelist;
1284 if (was_on_whitelist) { 1299 if (was_on_whitelist) {
1285 // TODO(asanka): Should sample whitelisted downloads based on 1300 RecordCountOfWhitelistedDownload(URL_WHITELIST);
1286 // service_->whitelist_sample_rate(). http://crbug.com/610924 1301 if (!ShouldSampleWhitelistedDownload()) {
1287 Finish(RequestOutcome::WHITELIST_HIT, SAFE); 1302 Finish(RequestOutcome::WHITELIST_HIT, SAFE);
1288 return; 1303 return;
1304 }
1305 sample_url_whitelist_ = true;
1306 } else {
1307 RecordCountOfWhitelistedDownload(NO_WHITELIST_MATCH);
1289 } 1308 }
1290 1309
1291 // Not on whitelist, so we are going to check with the SafeBrowsing 1310 // Not on whitelist, so we are going to check with the SafeBrowsing
1292 // backend. 1311 // backend.
1293 SendRequest(); 1312 SendRequest();
1294 } 1313 }
1295 1314
1296 void SendRequest() { 1315 void SendRequest() {
1297 DVLOG(2) << __FUNCTION__; 1316 DVLOG(2) << __FUNCTION__;
1298 DCHECK_CURRENTLY_ON(BrowserThread::UI); 1317 DCHECK_CURRENTLY_ON(BrowserThread::UI);
1299 1318
1300 ClientDownloadRequest request; 1319 ClientDownloadRequest request;
1301 request.set_download_type(ClientDownloadRequest::PPAPI_SAVE_REQUEST); 1320 request.set_download_type(ClientDownloadRequest::PPAPI_SAVE_REQUEST);
1302 ClientDownloadRequest::Resource* resource = request.add_resources(); 1321 ClientDownloadRequest::Resource* resource = request.add_resources();
1303 resource->set_type(ClientDownloadRequest::PPAPI_DOCUMENT); 1322 resource->set_type(ClientDownloadRequest::PPAPI_DOCUMENT);
1304 resource->set_url(requestor_url_.spec()); 1323 resource->set_url(requestor_url_.spec());
1305 request.set_url(requestor_url_.spec()); 1324 request.set_url(requestor_url_.spec());
1306 request.set_file_basename(supported_path_.BaseName().AsUTF8Unsafe()); 1325 request.set_file_basename(supported_path_.BaseName().AsUTF8Unsafe());
1307 request.set_length(0); 1326 request.set_length(0);
1308 request.mutable_digests()->set_md5(std::string()); 1327 request.mutable_digests()->set_md5(std::string());
1328 request.set_skipped_url_whitelist(sample_url_whitelist_);
1329 // Download protection does not check certificate whitelist for PPAPI
1330 // downloads.
1331 request.set_skipped_certificate_whitelist(false);
1309 for (const auto& alternate_extension : alternate_extensions_) { 1332 for (const auto& alternate_extension : alternate_extensions_) {
1310 if (alternate_extension.empty()) 1333 if (alternate_extension.empty())
1311 continue; 1334 continue;
1312 DCHECK_EQ(base::FilePath::kExtensionSeparator, alternate_extension[0]); 1335 DCHECK_EQ(base::FilePath::kExtensionSeparator, alternate_extension[0]);
1313 *(request.add_alternate_extensions()) = 1336 *(request.add_alternate_extensions()) =
1314 base::FilePath(alternate_extension).AsUTF8Unsafe(); 1337 base::FilePath(alternate_extension).AsUTF8Unsafe();
1315 } 1338 }
1316 if (supported_path_ != default_file_path_) { 1339 if (supported_path_ != default_file_path_) {
1317 *(request.add_alternate_extensions()) = 1340 *(request.add_alternate_extensions()) =
1318 base::FilePath(default_file_path_.FinalExtension()).AsUTF8Unsafe(); 1341 base::FilePath(default_file_path_.FinalExtension()).AsUTF8Unsafe();
(...skipping 132 matching lines...) Expand 10 before | Expand all | Expand 10 after
1451 // Time request was started. 1474 // Time request was started.
1452 const base::TimeTicks start_time_; 1475 const base::TimeTicks start_time_;
1453 1476
1454 // A download path that is supported by SafeBrowsing. This is determined by 1477 // A download path that is supported by SafeBrowsing. This is determined by
1455 // invoking GetSupportedFilePath(). If non-empty, 1478 // invoking GetSupportedFilePath(). If non-empty,
1456 // IsCheckedBinaryFile(supported_path_) is always true. This 1479 // IsCheckedBinaryFile(supported_path_) is always true. This
1457 // path is therefore used as the download target when sending the SafeBrowsing 1480 // path is therefore used as the download target when sending the SafeBrowsing
1458 // ping. 1481 // ping.
1459 const base::FilePath supported_path_; 1482 const base::FilePath supported_path_;
1460 1483
1484 bool sample_url_whitelist_;
1485
1486 bool is_extended_reporting_;
1487
1488 bool is_incognito_;
1489
1461 base::WeakPtrFactory<PPAPIDownloadRequest> weakptr_factory_; 1490 base::WeakPtrFactory<PPAPIDownloadRequest> weakptr_factory_;
1462 1491
1463 DISALLOW_COPY_AND_ASSIGN(PPAPIDownloadRequest); 1492 DISALLOW_COPY_AND_ASSIGN(PPAPIDownloadRequest);
1464 }; 1493 };
1465 1494
1466 DownloadProtectionService::DownloadProtectionService( 1495 DownloadProtectionService::DownloadProtectionService(
1467 SafeBrowsingService* sb_service) 1496 SafeBrowsingService* sb_service)
1468 : request_context_getter_(sb_service ? sb_service->url_request_context() 1497 : request_context_getter_(sb_service ? sb_service->url_request_context()
1469 : nullptr), 1498 : nullptr),
1470 enabled_(false), 1499 enabled_(false),
(...skipping 85 matching lines...) Expand 10 before | Expand all | Expand 10 after
1556 // UNKNOWN types properly. http://crbug.com/581044 1585 // UNKNOWN types properly. http://crbug.com/581044
1557 return (CheckClientDownloadRequest::IsSupportedDownload( 1586 return (CheckClientDownloadRequest::IsSupportedDownload(
1558 item, target_path, &reason, &type) && 1587 item, target_path, &reason, &type) &&
1559 (ClientDownloadRequest::CHROME_EXTENSION != type)); 1588 (ClientDownloadRequest::CHROME_EXTENSION != type));
1560 } 1589 }
1561 1590
1562 void DownloadProtectionService::CheckPPAPIDownloadRequest( 1591 void DownloadProtectionService::CheckPPAPIDownloadRequest(
1563 const GURL& requestor_url, 1592 const GURL& requestor_url,
1564 const base::FilePath& default_file_path, 1593 const base::FilePath& default_file_path,
1565 const std::vector<base::FilePath::StringType>& alternate_extensions, 1594 const std::vector<base::FilePath::StringType>& alternate_extensions,
1595 Profile* profile,
1566 const CheckDownloadCallback& callback) { 1596 const CheckDownloadCallback& callback) {
1567 DVLOG(1) << __FUNCTION__ << " url:" << requestor_url 1597 DVLOG(1) << __FUNCTION__ << " url:" << requestor_url
1568 << " default_file_path:" << default_file_path.value(); 1598 << " default_file_path:" << default_file_path.value();
1569 std::unique_ptr<PPAPIDownloadRequest> request(new PPAPIDownloadRequest( 1599 std::unique_ptr<PPAPIDownloadRequest> request(new PPAPIDownloadRequest(
1570 requestor_url, default_file_path, alternate_extensions, callback, this, 1600 requestor_url, default_file_path, alternate_extensions, profile, callback,
1571 database_manager_)); 1601 this, database_manager_));
1572 PPAPIDownloadRequest* request_copy = request.get(); 1602 PPAPIDownloadRequest* request_copy = request.get();
1573 auto insertion_result = ppapi_download_requests_.insert( 1603 auto insertion_result = ppapi_download_requests_.insert(
1574 std::make_pair(request_copy, std::move(request))); 1604 std::make_pair(request_copy, std::move(request)));
1575 DCHECK(insertion_result.second); 1605 DCHECK(insertion_result.second);
1576 insertion_result.first->second->Start(); 1606 insertion_result.first->second->Start();
1577 } 1607 }
1578 1608
1579 DownloadProtectionService::ClientDownloadRequestSubscription 1609 DownloadProtectionService::ClientDownloadRequestSubscription
1580 DownloadProtectionService::RegisterClientDownloadRequestCallback( 1610 DownloadProtectionService::RegisterClientDownloadRequestCallback(
1581 const ClientDownloadRequestCallback& callback) { 1611 const ClientDownloadRequestCallback& callback) {
(...skipping 145 matching lines...) Expand 10 before | Expand all | Expand 10 after
1727 GURL DownloadProtectionService::GetDownloadRequestUrl() { 1757 GURL DownloadProtectionService::GetDownloadRequestUrl() {
1728 GURL url(kDownloadRequestUrl); 1758 GURL url(kDownloadRequestUrl);
1729 std::string api_key = google_apis::GetAPIKey(); 1759 std::string api_key = google_apis::GetAPIKey();
1730 if (!api_key.empty()) 1760 if (!api_key.empty())
1731 url = url.Resolve("?key=" + net::EscapeQueryParamValue(api_key, true)); 1761 url = url.Resolve("?key=" + net::EscapeQueryParamValue(api_key, true));
1732 1762
1733 return url; 1763 return url;
1734 } 1764 }
1735 1765
1736 } // namespace safe_browsing 1766 } // namespace safe_browsing
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698