Skip cert whitelist checking if download URL matches whitelist and sampled

chrome/browser/safe_browsing/download_protection_service_unittest.cc

 // Copyright (c) 2012 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/download_protection_service.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <map>
@@ skipping 56 matching lines @@
 using ::testing::Invoke;
 using ::testing::Mock;
 using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::ReturnRef;
 using ::testing::SaveArg;
 using ::testing::StrictMock;
 using ::testing::_;
 using base::RunLoop;
 using content::BrowserThread;
+
 namespace safe_browsing {
+
 namespace {
+
 // A SafeBrowsingDatabaseManager implementation that returns a fixed result for
 // a given URL.
 class MockSafeBrowsingDatabaseManager : public TestSafeBrowsingDatabaseManager {
  public:
   MockSafeBrowsingDatabaseManager() {}
 
   MOCK_METHOD1(MatchDownloadWhitelistUrl, bool(const GURL&));
   MOCK_METHOD1(MatchDownloadWhitelistString, bool(const std::string&));
   MOCK_METHOD2(CheckDownloadUrl, bool(
       const std::vector<GURL>& url_chain,
@@ skipping 93 matching lines @@
   int WaitForRequest() {
     run_loop_.Run();
     return fetcher_id_;
   }
 
  private:
   net::TestURLFetcherFactory* factory_;
   int fetcher_id_;
   RunLoop run_loop_;
 };
+
 }  // namespace
 
 ACTION_P(SetCertificateContents, contents) {
   arg1->add_certificate_chain()->add_element()->set_certificate(contents);
 }
 
 ACTION_P(SetDosHeaderContents, contents) {
   arg2->mutable_pe_headers()->set_dos_header(contents);
   return true;
 }
 
-ACTION_P(TrustSignature, certificate_file) {
+ACTION_P(TrustSignature, contents) {
   arg1->set_trusted(true);
   // Add a certificate chain.  Note that we add the certificate twice so that
   // it appears as its own issuer.
-  std::string cert_data;
-  ASSERT_TRUE(base::ReadFileToString(certificate_file, &cert_data));
+
   ClientDownloadRequest_CertificateChain* chain =
       arg1->add_certificate_chain();
-  chain->add_element()->set_certificate(cert_data);
-  chain->add_element()->set_certificate(cert_data);
+  chain->add_element()->set_certificate(contents.data(), contents.size());
+  chain->add_element()->set_certificate(contents.data(), contents.size());
 }
 
 // We can't call OnSafeBrowsingResult directly because SafeBrowsingCheck does
 // not have any copy constructor which means it can't be stored in a callback
 // easily.  Note: check will be deleted automatically when the callback is
 // deleted.
 void OnSafeBrowsingResult(
     LocalSafeBrowsingDatabaseManager::SafeBrowsingCheck* check) {
   check->OnSafeBrowsingResult();
 }
@@ skipping 554 matching lines @@
     &item,
     std::vector<std::string>(),   // empty url_chain
     "http://www.google.com/",     // referrer
     FILE_PATH_LITERAL("a.tmp"),   // tmp_path
     FILE_PATH_LITERAL("a.exe"));  // final_path
   EXPECT_CALL(*binary_feature_extractor_.get(), CheckSignature(tmp_path_, _))
       .Times(4);
   EXPECT_CALL(*binary_feature_extractor_.get(),
               ExtractImageFeatures(
                   tmp_path_, BinaryFeatureExtractor::kDefaultOptions, _, _))
-      .Times(4);
+      .Times(6);
   // Assume http://www.whitelist.com/a.exe is on the whitelist.
   EXPECT_CALL(*sb_service_->mock_database_manager(),
               MatchDownloadWhitelistUrl(_)).Times(0);
   EXPECT_CALL(*sb_service_->mock_database_manager(),
               MatchDownloadWhitelistUrl(GURL("http://www.whitelist.com/a.exe")))
       .WillRepeatedly(Return(true));
   url_chain_.push_back(GURL("http://www.whitelist.com/a.exe"));
   // Set sample rate to 1.00, so download_service_ will always send download
   // pings for whitelisted downloads.
   SetWhitelistedDownloadSampleRate(1.00);
 
   {
     // Case (1): is_extended_reporting && is_incognito.
     //           ClientDownloadRequest should NOT be sent.
     SetExtendedReportingPreference(true);
     EXPECT_CALL(item, GetBrowserContext())
         .WillRepeatedly(Return(profile_->GetOffTheRecordProfile()));
     RunLoop run_loop;
     download_service_->CheckClientDownload(
         &item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
                           base::Unretained(this), run_loop.QuitClosure()));
     run_loop.Run();
     EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
     EXPECT_FALSE(HasClientDownloadRequest());
   }
   {
-    // Case (2): is_extended_reporting && !is_incognito.
-    //           ClientDownloadRequest should be sent.
+    // Case (2): !is_extended_reporting && is_incognito.
+    //           ClientDownloadRequest should NOT be sent.
+    SetExtendedReportingPreference(false);
+    EXPECT_CALL(item, GetBrowserContext())
+        .WillRepeatedly(Return(profile_->GetOffTheRecordProfile()));
+    RunLoop run_loop;
+    download_service_->CheckClientDownload(
+        &item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
+                          base::Unretained(this), run_loop.QuitClosure()));
+    run_loop.Run();
+    EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
+    EXPECT_FALSE(HasClientDownloadRequest());
+  }
+  {
+    // Case (3): !is_extended_reporting && !is_incognito.
+    //           ClientDownloadRequest should NOT be sent.
     EXPECT_CALL(item, GetBrowserContext())
         .WillRepeatedly(Return(profile_.get()));
     RunLoop run_loop;
+    download_service_->CheckClientDownload(
+        &item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
+                          base::Unretained(this), run_loop.QuitClosure()));
+    run_loop.Run();
+    EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
+    EXPECT_FALSE(HasClientDownloadRequest());
+  }
+  {
+    // Case (4): is_extended_reporting && !is_incognito &&
+    //           Download matches URL whitelist.
+    //           ClientDownloadRequest should be sent.
+    SetExtendedReportingPreference(true);
+    EXPECT_CALL(item, GetBrowserContext())
+        .WillRepeatedly(Return(profile_.get()));
+    RunLoop run_loop;
     download_service_->CheckClientDownload(
         &item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
                           base::Unretained(this), run_loop.QuitClosure()));
     run_loop.Run();
     EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
     ASSERT_TRUE(HasClientDownloadRequest());
     EXPECT_TRUE(GetClientDownloadRequest()->skipped_url_whitelist());
+    EXPECT_FALSE(GetClientDownloadRequest()->skipped_certificate_whitelist());
     ClearClientDownloadRequest();
   }
+
+  // Setup trusted and whitelisted certificates for test cases (5) and (6).
+  scoped_refptr<net::X509Certificate> test_cert(
+      ReadTestCertificate("test_cn.pem"));
+  ASSERT_TRUE(test_cert.get());
+  std::string test_cert_der;
+  net::X509Certificate::GetDEREncoded(test_cert->os_cert_handle(),
+                                      &test_cert_der);
+  EXPECT_CALL(*binary_feature_extractor_.get(), CheckSignature(tmp_path_, _))
+        .WillRepeatedly(TrustSignature(test_cert_der));
+  EXPECT_CALL(*sb_service_->mock_database_manager(),
+              MatchDownloadWhitelistString(_))
+      .WillRepeatedly(Return(true));
+
   {
-    // Case (3): !is_extended_reporting && is_incognito.
-    //           ClientDownloadRequest should NOT be sent.
-    SetExtendedReportingPreference(false);
+    // Case (5): is_extended_reporting && !is_incognito &&
+    //           Download matches certificate whitelist.
+    //           ClientDownloadRequest should be sent.
     EXPECT_CALL(item, GetBrowserContext())
-        .WillRepeatedly(Return(profile_->GetOffTheRecordProfile()));
+        .WillRepeatedly(Return(profile_.get()));
+    EXPECT_CALL(*sb_service_->mock_database_manager(),
+                MatchDownloadWhitelistUrl(GURL("http://www.whitelist.com/a.exe")))

[Nathan Parker, 2016/08/05 17:27:10]

nit: > 80 char line?  Below as well.

[Jialiu Lin, 2016/08/05 17:32:25]

Oops, fixed.

+        .WillRepeatedly(Return(false));
     RunLoop run_loop;
     download_service_->CheckClientDownload(
         &item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
                           base::Unretained(this), run_loop.QuitClosure()));
     run_loop.Run();
     EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
-    EXPECT_FALSE(HasClientDownloadRequest());
+    ASSERT_TRUE(HasClientDownloadRequest());
+    EXPECT_FALSE(GetClientDownloadRequest()->skipped_url_whitelist());
+    EXPECT_TRUE(GetClientDownloadRequest()->skipped_certificate_whitelist());
+    ClearClientDownloadRequest();
   }
   {
-    // Case (4): !is_extended_reporting && !is_incognito.
-    //           ClientDownloadRequest should NOT be sent.
+    // Case (6): is_extended_reporting && !is_incognito &&
+    //           Download matches both URL and certificate whitelists.
+    //           ClientDownloadRequest should be sent.
     EXPECT_CALL(item, GetBrowserContext())
         .WillRepeatedly(Return(profile_.get()));
+    EXPECT_CALL(*sb_service_->mock_database_manager(),
+                MatchDownloadWhitelistUrl(GURL("http://www.whitelist.com/a.exe")))
+        .WillRepeatedly(Return(true));
     RunLoop run_loop;
     download_service_->CheckClientDownload(
         &item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
                           base::Unretained(this), run_loop.QuitClosure()));
     run_loop.Run();
     EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
-    EXPECT_FALSE(HasClientDownloadRequest());
+    ASSERT_TRUE(HasClientDownloadRequest());
+    EXPECT_TRUE(GetClientDownloadRequest()->skipped_url_whitelist());
+    // Since download matches URL whitelist and gets sampled, no need to
+    // do certificate whitelist checking and sampling.
+    EXPECT_FALSE(GetClientDownloadRequest()->skipped_certificate_whitelist());
+    ClearClientDownloadRequest();
   }
 }
 
 TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSampledFile) {
   // Server response will be discarded.
   net::FakeURLFetcherFactory factory(NULL);
   PrepareResponse(
       &factory, ClientDownloadResponse::DANGEROUS, net::HTTP_OK,
       net::URLRequestStatus::SUCCESS);
 
@@ skipping 1436 matching lines @@
       &item, base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
                         base::Unretained(this), run_loop.QuitClosure()));
   run_loop.Run();
 
   EXPECT_FALSE(HasClientDownloadRequest());
   // Overriden by flag:
   EXPECT_TRUE(IsResult(DownloadProtectionService::DANGEROUS));
 }
 
 }  // namespace safe_browsing