Handle non-success results for Deep Scanning of uploads

Bug: 1042319
Change-Id: Ie33e4f358d35bb6ed5314e19035536c4860afbd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003349
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733365}

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.cc

@@ -298,6 +298,29 @@ bool DeepScanningDialogDelegate::FileTypeSupported(const bool for_malware_scan,
   return false;
 }
 
+bool DeepScanningDialogDelegate::ResultShouldAllowDataUse(
+    BinaryUploadService::Result result) {
+  // Keep this implemented as a switch instead of a simpler if statement so that
+  // new values added to BinaryUploadService::Result cause a compiler error.
+  switch (result) {
+    case BinaryUploadService::Result::SUCCESS:
+    case BinaryUploadService::Result::UPLOAD_FAILURE:
+    case BinaryUploadService::Result::TIMEOUT:
+    case BinaryUploadService::Result::FAILED_TO_GET_TOKEN:
+    // UNAUTHORIZED allows data usage since it's a result only obtained if the
+    // browser is not authorized to perform deep scanning. It does not make
+    // sense to block data in this situation since no actual scanning of the
+    // data was performed, so it's allowed.
+    case BinaryUploadService::Result::UNAUTHORIZED:
+    case BinaryUploadService::Result::UNKNOWN:
+      return true;
+
+    case BinaryUploadService::Result::FILE_TOO_LARGE:
+    case BinaryUploadService::Result::FILE_ENCRYPTED:
+      return false;
+  }
+}
+
 // static
 bool DeepScanningDialogDelegate::IsEnabled(Profile* profile,
                                            GURL url,
@@ -443,8 +466,8 @@ void DeepScanningDialogDelegate::StringRequestCallback(
       content_size, result, response);
 
   text_request_complete_ = true;
-  bool text_complies = (result == BinaryUploadService::Result::SUCCESS &&
-                        DlpTriggeredRulesOK(response.dlp_scan_verdict()));
+  bool text_complies = ResultShouldAllowDataUse(result) &&
+                       DlpTriggeredRulesOK(response.dlp_scan_verdict());
   std::fill(result_.text_results.begin(), result_.text_results.end(),
             text_complies);
   MaybeCompleteScanRequest();
@@ -475,9 +498,7 @@ void DeepScanningDialogDelegate::CompleteFileRequestCallback(
                      MalwareDeepScanningVerdict::MALWARE;
   }
 
-  bool file_complies = (result == BinaryUploadService::Result::SUCCESS ||
-                        result == BinaryUploadService::Result::UNAUTHORIZED) &&
-                       dlp_ok && malware_ok;
+  bool file_complies = ResultShouldAllowDataUse(result) && dlp_ok && malware_ok;
   result_.paths_results[index] = file_complies;
 
   ++file_result_count_;

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.h

@@ -177,6 +177,10 @@ class DeepScanningDialogDelegate {
                                 const bool for_dlp_scan,
                                 const base::FilePath& path);
 
+  // Determines if a request result should be used to allow a data use or to
+  // block it.
+  static bool ResultShouldAllowDataUse(BinaryUploadService::Result result);
+
  protected:
   DeepScanningDialogDelegate(
       content::WebContents* web_contents,

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.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"
 #include "chrome/browser/safe_browsing/dm_token_utils.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -1392,6 +1393,55 @@ TEST_F(DeepScanningDialogDelegateAuditOnlyTest, UnsupportedTypeAndDLPFailure) {
   EXPECT_TRUE(called);
 }
 
+// TODO(crbug/1041890): This test should not depend on
+// DeepScanningDialogDelegateAuditOnlyTest. Refactor this by moving common
+// functionalities to BaseTest or util classes/functions.
+class DeepScanningDialogDelegateResultHandlingTest
+    : public DeepScanningDialogDelegateAuditOnlyTest,
+      public testing::WithParamInterface<BinaryUploadService::Result> {};
+
+TEST_P(DeepScanningDialogDelegateResultHandlingTest, Test) {
+  GURL url(kTestUrl);
+  DeepScanningDialogDelegate::Data data;
+  FakeDeepScanningDialogDelegate::SetResponseResult(GetParam());
+  ASSERT_TRUE(DeepScanningDialogDelegate::IsEnabled(profile(), url, &data));
+
+  data.paths.emplace_back(FILE_PATH_LITERAL("/tmp/foo.txt"));
+
+  bool called = false;
+  DeepScanningDialogDelegate::ShowForWebContents(
+      contents(), std::move(data),
+      base::BindOnce(
+          [](bool* called, const DeepScanningDialogDelegate::Data& data,
+             const DeepScanningDialogDelegate::Result& result) {
+            EXPECT_EQ(0u, data.text.size());
+            EXPECT_EQ(1u, data.paths.size());
+            EXPECT_EQ(0u, result.text_results.size());
+            EXPECT_EQ(1u, result.paths_results.size());
+
+            bool expected =
+                DeepScanningDialogDelegate::ResultShouldAllowDataUse(
+                    GetParam());
+            EXPECT_EQ(expected, result.paths_results[0]);
+            *called = true;
+          },
+          &called));
+  RunUntilDone();
+  EXPECT_TRUE(called);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    Tests,
+    DeepScanningDialogDelegateResultHandlingTest,
+    testing::Values(BinaryUploadService::Result::UNKNOWN,
+                    BinaryUploadService::Result::SUCCESS,
+                    BinaryUploadService::Result::UPLOAD_FAILURE,
+                    BinaryUploadService::Result::TIMEOUT,
+                    BinaryUploadService::Result::FILE_TOO_LARGE,
+                    BinaryUploadService::Result::FAILED_TO_GET_TOKEN,
+                    BinaryUploadService::Result::UNAUTHORIZED,
+                    BinaryUploadService::Result::FILE_ENCRYPTED));
+
 }  // namespace
 
 }  // namespace safe_browsing

chrome/browser/safe_browsing/cloud_content_scanning/fake_deep_scanning_dialog_delegate.cc

@@ -6,9 +6,13 @@
 
 #include <base/callback.h>
 #include <base/logging.h>
+#include "chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.h"
 
 namespace safe_browsing {
 
+BinaryUploadService::Result FakeDeepScanningDialogDelegate::result_ =
+    BinaryUploadService::Result::SUCCESS;
+
 FakeDeepScanningDialogDelegate::FakeDeepScanningDialogDelegate(
     base::RepeatingClosure delete_closure,
     StatusCallback status_callback,
@@ -30,6 +34,12 @@ FakeDeepScanningDialogDelegate::~FakeDeepScanningDialogDelegate() {
     delete_closure_.Run();
 }
 
+// static
+void FakeDeepScanningDialogDelegate::SetResponseResult(
+    BinaryUploadService::Result result) {
+  result_ = result;
+}
+
 // static
 std::unique_ptr<DeepScanningDialogDelegate>
 FakeDeepScanningDialogDelegate::Create(
@@ -117,9 +127,9 @@ void FakeDeepScanningDialogDelegate::Response(
                                             ? DeepScanningClientResponse()
                                             : status_callback_.Run(path);
   if (path.empty())
-    StringRequestCallback(BinaryUploadService::Result::SUCCESS, response);
+    StringRequestCallback(result_, response);
   else
-    FileRequestCallback(path, BinaryUploadService::Result::SUCCESS, response);
+    FileRequestCallback(path, result_, response);
 }
 
 void FakeDeepScanningDialogDelegate::PrepareFileRequest(

chrome/browser/safe_browsing/cloud_content_scanning/fake_deep_scanning_dialog_delegate.h

@@ -79,6 +79,9 @@ class FakeDeepScanningDialogDelegate : public DeepScanningDialogDelegate {
       const std::string& rule_name,
       DlpDeepScanningVerdict::TriggeredRule::Action action);
 
+  // Sets the BinaryUploadService::Result to use in the next response callback.
+  static void SetResponseResult(BinaryUploadService::Result result);
+
  private:
   // Simulates a response from the binary upload service.  the |path| argument
   // is used to call |status_callback_| to determine if the path should succeed
@@ -96,6 +99,7 @@ class FakeDeepScanningDialogDelegate : public DeepScanningDialogDelegate {
       std::unique_ptr<BinaryUploadService::Request> request) override;
   bool CloseTabModalDialog() override;
 
+  static BinaryUploadService::Result result_;
   base::RepeatingClosure delete_closure_;
   StatusCallback status_callback_;
   EncryptionStatusCallback encryption_callback_;