diff options
4 files changed, 50 insertions, 15 deletions
diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc index afb797d..ce10f13 100644 --- a/chrome/browser/safe_browsing/download_protection_service.cc +++ b/chrome/browser/safe_browsing/download_protection_service.cc @@ -186,26 +186,27 @@ class DownloadProtectionService::CheckClientDownloadRequest << info_.download_url_chain.back() << ": success=" << source->GetStatus().is_success() << " response_code=" << source->GetResponseCode(); - DownloadCheckResultReason reason = REASON_MAX; - reason = REASON_SERVER_PING_FAILED; + DownloadCheckResultReason reason = REASON_SERVER_PING_FAILED; + DownloadCheckResult result = SAFE; if (source->GetStatus().is_success() && RC_REQUEST_OK == source->GetResponseCode()) { + ClientDownloadResponse response; std::string data; - source->GetResponseAsString(&data); - if (data.size() > 0) { - // For now no matter what we'll always say the download is safe. - // TODO(noelutz): Parse the response body to see exactly what's going - // on. + bool got_data = source->GetResponseAsString(&data); + DCHECK(got_data); + if (!response.ParseFromString(data)) { reason = REASON_INVALID_RESPONSE_PROTO; + } else if (response.verdict() == ClientDownloadResponse::DANGEROUS) { + reason = REASON_DOWNLOAD_DANGEROUS; + result = DANGEROUS; + } else { + reason = REASON_DOWNLOAD_SAFE; } } - - if (reason != REASON_MAX) { - RecordStats(reason); - } // We don't need the fetcher anymore. fetcher_.reset(); - FinishRequest(SAFE); + RecordStats(reason); + FinishRequest(result); } private: diff --git a/chrome/browser/safe_browsing/download_protection_service.h b/chrome/browser/safe_browsing/download_protection_service.h index a2d87b5..a18935d 100644 --- a/chrome/browser/safe_browsing/download_protection_service.h +++ b/chrome/browser/safe_browsing/download_protection_service.h @@ -51,7 +51,7 @@ class DownloadProtectionService { enum DownloadCheckResult { SAFE, - MALICIOUS, + DANGEROUS, // In the future we may introduce a third category which corresponds to // suspicious downloads that are not known to be malicious. }; @@ -98,6 +98,8 @@ class DownloadProtectionService { REASON_INVALID_RESPONSE_PROTO, REASON_NOT_BINARY_FILE, REASON_REQUEST_CANCELED, + REASON_DOWNLOAD_DANGEROUS, + REASON_DOWNLOAD_SAFE, REASON_MAX // Always add new values before this one. }; diff --git a/chrome/browser/safe_browsing/download_protection_service_unittest.cc b/chrome/browser/safe_browsing/download_protection_service_unittest.cc index be3ca1e..2b12627 100644 --- a/chrome/browser/safe_browsing/download_protection_service_unittest.cc +++ b/chrome/browser/safe_browsing/download_protection_service_unittest.cc @@ -224,10 +224,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadFetchFailed) { } TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) { + ClientDownloadResponse response; + response.set_verdict(ClientDownloadResponse::SAFE); FakeURLFetcherFactory factory; // Empty response means SAFE. factory.SetFakeResponse( - DownloadProtectionService::kDownloadRequestUrl, "", true); + DownloadProtectionService::kDownloadRequestUrl, + response.SerializeAsString(), + true); EXPECT_CALL(*sb_service_, MatchDownloadWhitelistUrl(_)) .WillRepeatedly(Return(false)); @@ -244,8 +248,11 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) { EXPECT_EQ(DownloadProtectionService::SAFE, result_); // Invalid response should be safe too. + response.Clear(); factory.SetFakeResponse( - DownloadProtectionService::kDownloadRequestUrl, "bla", true); + DownloadProtectionService::kDownloadRequestUrl, + response.SerializePartialAsString(), + true); download_service_->CheckClientDownload( info, @@ -253,6 +260,20 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) { base::Unretained(this))); msg_loop_.Run(); EXPECT_EQ(DownloadProtectionService::SAFE, result_); + + // If the response is dangerous the result should also be marked as dangerous. + response.set_verdict(ClientDownloadResponse::DANGEROUS); + factory.SetFakeResponse( + DownloadProtectionService::kDownloadRequestUrl, + response.SerializeAsString(), + true); + + download_service_->CheckClientDownload( + info, + base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback, + base::Unretained(this))); + msg_loop_.Run(); + EXPECT_EQ(DownloadProtectionService::DANGEROUS, result_); } TEST_F(DownloadProtectionServiceTest, CheckClientDownloadValidateRequest) { diff --git a/chrome/common/safe_browsing/csd.proto b/chrome/common/safe_browsing/csd.proto index 1811c76..a95bad8 100644 --- a/chrome/common/safe_browsing/csd.proto +++ b/chrome/common/safe_browsing/csd.proto @@ -146,3 +146,14 @@ message ClientDownloadRequest { // True if the download was user initiated. optional bool user_initiated = 6; } + +message ClientDownloadResponse { + enum Verdict { + // Download is considered safe. + SAFE = 0; + // Download is considered dangerous. Chrome should show a warning to the + // user. + DANGEROUS = 1; + } + required Verdict verdict = 1; +} |