Add digests to files sent for deep scanning

This CL adds the digest field to the metadata uploaded when deep
scanning a file. This allows for a very early return before any
processing of file contents has occurred at all, in cases where the
file is not unique.

Fixed: 1056929
Change-Id: I75e703a516cbb06b4f1aa480fd46c66ac20e37c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090607
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Dominique Fauteux-Chapleau <domfc@chromium.org>
Reviewed-by: Bettina Dea <bdea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747886}

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.cc

@@ -413,6 +413,10 @@ void BinaryUploadService::Request::set_filename(const std::string& filename) {
   deep_scanning_request_.set_filename(filename);
 }
 
+void BinaryUploadService::Request::set_digest(const std::string& digest) {
+  deep_scanning_request_.set_digest(digest);
+}
+
 void BinaryUploadService::Request::FinishRequest(
     Result result,
     DeepScanningClientResponse response) {

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.h

@@ -126,6 +126,7 @@ class BinaryUploadService {
     void set_dm_token(const std::string& token);
     void set_request_token(const std::string& token);
     void set_filename(const std::string& filename);
+    void set_digest(const std::string& digest);
 
     // Finish the request, with the given |result| and |response| from the
     // server.

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.cc

@@ -224,29 +224,6 @@ bool* UIEnabledStorage() {
 
 }  // namespace
 
-// A BinaryUploadService::Request implementation that gets the data to scan
-// from the contents of a file.
-class DeepScanningDialogDelegate::FileSourceRequest
-    : public BinaryUploadService::Request {
- public:
-  FileSourceRequest(base::WeakPtr<DeepScanningDialogDelegate> delegate,
-                    base::FilePath path,
-                    BinaryUploadService::Callback callback);
-  FileSourceRequest(const FileSourceRequest&) = delete;
-  FileSourceRequest& operator=(const FileSourceRequest&) = delete;
-  ~FileSourceRequest() override = default;
-
- private:
-  // BinaryUploadService::Request implementation.
-  void GetRequestData(DataCallback callback) override;
-
-  void OnGotFileContents(DataCallback callback, FileContents file_contents);
-
-  base::WeakPtr<DeepScanningDialogDelegate> delegate_;
-  base::FilePath path_;
-  base::WeakPtrFactory<FileSourceRequest> weakptr_factory_{this};
-};
-
 DeepScanningDialogDelegate::FileSourceRequest::FileSourceRequest(
     base::WeakPtr<DeepScanningDialogDelegate> delegate,
     base::FilePath path,
@@ -257,6 +234,8 @@ DeepScanningDialogDelegate::FileSourceRequest::FileSourceRequest(
   set_filename(path_.BaseName().AsUTF8Unsafe());
 }
 
+DeepScanningDialogDelegate::FileSourceRequest::~FileSourceRequest() = default;
+
 void DeepScanningDialogDelegate::FileSourceRequest::GetRequestData(
     DataCallback callback) {
   base::ThreadPool::PostTaskAndReplyWithResult(
@@ -274,6 +253,9 @@ void DeepScanningDialogDelegate::FileSourceRequest::OnGotFileContents(
     delegate_->SetFileInfo(path_, std::move(file_contents.sha256),
                            file_contents.size);
 
+  set_digest(base::HexEncode(file_contents.sha256.data(),
+                             file_contents.sha256.size()));
+
   std::move(callback).Run(file_contents.result, file_contents.data);
 }
 

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.h

@@ -224,6 +224,28 @@ class DeepScanningDialogDelegate {
   // Callback used by FileSourceRequest to read file data on a blocking thread.
   static FileContents GetFileContentsSHA256Blocking(const base::FilePath& path);
 
+  // A BinaryUploadService::Request implementation that gets the data to scan
+  // from the contents of a file.
+  class FileSourceRequest : public BinaryUploadService::Request {
+   public:
+    FileSourceRequest(base::WeakPtr<DeepScanningDialogDelegate> delegate,
+                      base::FilePath path,
+                      BinaryUploadService::Callback callback);
+    FileSourceRequest(const FileSourceRequest&) = delete;
+    FileSourceRequest& operator=(const FileSourceRequest&) = delete;
+    ~FileSourceRequest() override;
+
+    // BinaryUploadService::Request implementation.
+    void GetRequestData(DataCallback callback) override;
+
+   private:
+    void OnGotFileContents(DataCallback callback, FileContents file_contents);
+
+    base::WeakPtr<DeepScanningDialogDelegate> delegate_;
+    base::FilePath path_;
+    base::WeakPtrFactory<FileSourceRequest> weakptr_factory_{this};
+  };
+
  protected:
   DeepScanningDialogDelegate(content::WebContents* web_contents,
                              Data data,
@@ -243,8 +265,6 @@ class DeepScanningDialogDelegate {
   }
 
  private:
-  class FileSourceRequest;
-
   // Uploads data for deep scanning.  Returns true if uploading is occurring in
   // the background and false if there is nothing to do.
   bool UploadData();

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate_unittest.cc

@@ -10,10 +10,12 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.h"
 #include "chrome/browser/safe_browsing/cloud_content_scanning/fake_deep_scanning_dialog_delegate.h"
@@ -1624,4 +1626,56 @@ TEST_F(DeepScanningFileContentsTest, LargeFiles) {
            BinaryUploadService::Result::FILE_TOO_LARGE);
 }
 
+TEST(DeepScanningFileSourceRequestTest, PopulatesDigest) {
+  base::test::TaskEnvironment task_environment;
+  std::string file_contents = "Normal file contents";
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  base::FilePath file_path = temp_dir.GetPath().AppendASCII("foo.doc");
+
+  // Create the file.
+  base::File file(file_path, base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+  file.WriteAtCurrentPos(file_contents.data(), file_contents.size());
+
+  DeepScanningDialogDelegate::FileSourceRequest request(nullptr, file_path,
+                                                        base::DoNothing());
+
+  base::RunLoop run_loop;
+  request.GetRequestData(base::BindLambdaForTesting(
+      [&run_loop](BinaryUploadService::Result result,
+                  const BinaryUploadService::Request::Data& data) {
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+
+  // printf "Normal file contents" | sha256sum |  tr '[:lower:]' '[:upper:]'
+  EXPECT_EQ(request.deep_scanning_request().digest(),
+            "29644C10BD036866FCFD2BDACFF340DB5DE47A90002D6AB0C42DE6A22C26158B");
+}
+
+TEST(DeepScanningFileSourceRequestTest, PopulatesFilename) {
+  base::test::TaskEnvironment task_environment;
+  std::string file_contents = "contents";
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  base::FilePath file_path = temp_dir.GetPath().AppendASCII("foo.doc");
+
+  // Create the file.
+  base::File file(file_path, base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+  file.WriteAtCurrentPos(file_contents.data(), file_contents.size());
+
+  DeepScanningDialogDelegate::FileSourceRequest request(nullptr, file_path,
+                                                        base::DoNothing());
+
+  base::RunLoop run_loop;
+  request.GetRequestData(base::BindLambdaForTesting(
+      [&run_loop](BinaryUploadService::Result result,
+                  const BinaryUploadService::Request::Data& data) {
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+
+  EXPECT_EQ(request.deep_scanning_request().filename(), "foo.doc");
+}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/download_protection/deep_scanning_request.cc

@@ -181,6 +181,10 @@ void DeepScanningRequest::Start() {
                      weak_ptr_factory_.GetWeakPtr()));
   request->set_filename(item_->GetTargetFilePath().BaseName().AsUTF8Unsafe());
 
+  std::string raw_digest_sha256 = item_->GetHash();
+  request->set_digest(
+      base::HexEncode(raw_digest_sha256.data(), raw_digest_sha256.size()));
+
   Profile* profile = Profile::FromBrowserContext(
       content::DownloadItemUtils::GetBrowserContext(item_));
 

chrome/browser/safe_browsing/download_protection/deep_scanning_request_unittest.cc

@@ -500,4 +500,24 @@ TEST_F(DeepScanningRequestTest, ShouldUploadItemByPolicy_MalwareListPolicy) {
   EXPECT_FALSE(DeepScanningRequest::ShouldUploadItemByPolicy(&item_));
 }
 
+TEST_F(DeepScanningRequestTest, PopulatesRequest) {
+  SetDlpPolicy(CHECK_UPLOADS_AND_DOWNLOADS);
+  SetMalwarePolicy(SEND_UPLOADS_AND_DOWNLOADS);
+
+  EnableFeatures({kMalwareScanEnabled, kContentComplianceEnabled});
+  DeepScanningRequest request(
+      &item_, DeepScanningRequest::DeepScanTrigger::TRIGGER_POLICY,
+      base::DoNothing(), &download_protection_service_);
+  request.Start();
+  EXPECT_EQ(download_protection_service_.GetFakeBinaryUploadService()
+                ->last_request()
+                .filename(),
+            "download.exe");
+  EXPECT_EQ(download_protection_service_.GetFakeBinaryUploadService()
+                ->last_request()
+                .digest(),
+            // Hex-encoding of 'hash'
+            "68617368");
+}
+
 }  // namespace safe_browsing

components/safe_browsing/core/proto/webprotect.proto

@@ -58,6 +58,9 @@ message DeepScanningClientRequest {
 
   // Name of file on user system (if applicable).
   optional string filename = 6;
+
+  // Sha256 digest of file.
+  optional string digest = 7;
 }
 
 // Malware-specific response given back for scanned content.