Disable download protection remote lookups (brave/brave-browser#4341)

[fmarier]

Fixes brave/brave-browser#4341.

Submitter Checklist:

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Verified that these changes build without errors on
  • Android
  • iOS
  • Linux
  • macOS
  • Windows
• Verified that these changes pass automated tests (unit, browser, security tests) on
  • iOS
  • Linux
  • macOS
  • Windows
• Verified that all lint errors/warnings are resolved (npm run lint)
• Ran git rebase master (if needed).
• Ran git rebase -i to squash commits (if needed).
• Tagged reviewers and labelled the pull request as needed.
• Request a security/privacy review as needed.
• Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone
• Public documentation has been updated as necessary. For instance:
  • https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • https://github.com/brave/brave-browser/wiki/Fingerprinting-Protection-Mode
  • https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • https://github.com/brave/brave-browser/wiki/Custom-Headers
  • https://github.com/brave/brave-browser/wiki/Web-compatibility-issues-with-tracking-protection
  • https://github.com/brave/brave-browser/wiki/QA-Guide

Test Plan:

To test that remote lookups are no longer done, go to https://testsafebrosing.appspot.com and verify that none of the Download Warnings files get blocked. Ideally, do this while looking at a MITM proxy to ensure that nothing is sent to sb-ssl.google.com or sb-ssl.brave.com.

To test that the "bad binary" list is still used against the download URL, I made a build which always flags download URLs as dangerous:

--- a/chrome/browser/safe_browsing/download_protection/download_url_sb_client.cc
+++ b/chrome/browser/safe_browsing/download_protection/download_url_sb_client.cc
@@ -58,7 +58,7 @@ void DownloadUrlSBClient::StartCheck() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!database_manager_.get() ||
       database_manager_->CheckDownloadUrl(url_chain_, this)) {
-    CheckDone(SB_THREAT_TYPE_SAFE);
+    CheckDone(SB_THREAT_TYPE_URL_BINARY_MALWARE);
   } else {
     // Add a reference to this object to prevent it from being destroyed
     // before url checking result is returned.
@@ -74,7 +74,7 @@ bool DownloadUrlSBClient::IsDangerous(SBThreatType threat_type) const {
 void DownloadUrlSBClient::OnCheckDownloadUrlResult(
     const std::vector<GURL>& url_chain,
     SBThreatType threat_type) {
-  CheckDone(threat_type);
+  CheckDone(SB_THREAT_TYPE_URL_BINARY_MALWARE);
   UMA_HISTOGRAM_TIMES("SB2.DownloadUrlCheckDuration",
                       base::TimeTicks::Now() - start_time_);
   Release();

and then verified that any download would be blocked.

Reviewer Checklist:

• New files have MPL-2.0 license header.
• Request a security/privacy review as needed.
• Adequate test coverage exists to prevent regressions
• Verify test plan is specified in PR before merging to source

After-merge Checklist:

• The associated issue milestone is set to the smallest version that the
  changes has landed on.
• All relevant documentation has been updated.
• Update https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)

[simonhong]

I think this issue can be resolved by below one patch.
If we can avoid patch by our overriding technique, it is more preferred.
However, we need more patch files and additional implementation by using BraveCheckClientDownloadRequest.
WDYT @bridiver ?

diff --git a/chrome/browser/safe_browsing/download_protection/check_client_download_request.cc b/chrome/browser/safe_browsing/download_protection/check_client_download_request.cc
index 59a28c8c72340677aaed45e042fe8ad499fa4fed..eb78bab6c36dad96c4edbb46b8c09bce6b891e1f 100644
--- a/chrome/browser/safe_browsing/download_protection/check_client_download_request.cc
+++ b/chrome/browser/safe_browsing/download_protection/check_client_download_request.cc
@@ -599,6 +599,8 @@ std::string CheckClientDownloadRequest::SanitizeUrl(const GURL& url) const {

 void CheckClientDownloadRequest::SendRequest() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  FinishRequest(DownloadCheckResult::UNKNOWN, REASON_PING_DISABLED);
+  return;

   if (item_->GetState() == download::DownloadItem::CANCELLED) {
     FinishRequest(DownloadCheckResult::UNKNOWN, REASON_DOWNLOAD_DESTROYED);

[bridiver]

I think this is better handled by blocking the request url in the network delegate which would not require any patching

[simonhong]

LGTM. (but, need owner's approve)

[iefremov]

some improvements are needed

[fmarier]

Thanks for all of your feedback @iefremov . I believe I have addressed everything.

I just re-rested manually and ran all of the tests successfully. This is ready for your final review.