ungoogled-software / ungoogled-chromium

Google Chromium, sans integration with Google
BSD 3-Clause "New" or "Revised" License
20.8k stars 844 forks source link

Enable blocking all downloads via managed policy #3000

Closed berkley4 closed 2 months ago

berkley4 commented 2 months ago

In a nutshell, this commit allows users to set the DownloadRestrictions managed policy key to either 0 or 3 to respectively permit or block all downloads.

The key values are as follows.....

0 = No special restrictions. Default. 1 = Block malicious downloads and dangerous file types. 2 = Block malicious downloads, uncommon or unwanted downloads and dangerous file types. 3 = Block all downloads. 4 = Block malicious downloads. Recommended.

In this commit, DownloadRestrictions values of 1, 2 and 4 are effectively neutered by disentangling them from safebrowsing and treating them as invalid. None of the reinstated code appears to be connected to safebrowsing.

The relevant section of chrome/browser/download/chrome_download_manager_delegate.cc will end up looking like this.....

bool ChromeDownloadManagerDelegate::ShouldBlockFile(
    download::DownloadItem* item,
    download::DownloadDangerType danger_type) const {
  DownloadPrefs::DownloadRestriction download_restriction =
      download_prefs_->download_restriction();

  switch (download_restriction) {
    case (DownloadPrefs::DownloadRestriction::NONE):
      return false;

    case (DownloadPrefs::DownloadRestriction::ALL_FILES):
      return true;

    // DownloadRestrictions policy key values 1, 2 and 4 treated as invalid
    case (DownloadPrefs::DownloadRestriction::POTENTIALLY_DANGEROUS_FILES):
    case (DownloadPrefs::DownloadRestriction::DANGEROUS_FILES):
    case (DownloadPrefs::DownloadRestriction::MALICIOUS_FILES):
    default:
      LOG(ERROR) << "Invalid download restriction value: "
                 << static_cast<int>(download_restriction);
  }

  return false;
}

I have tested DownloadRestrictions values of 0-5, with 0 and 3 behaving as expected. Running without a policy file also behaves as expected, same as 0 (ie there is no intrinsic change in behaviour from this commit).

Values 1, 2 and 4 behave the same as 5, emitting a non-fatal "Invalid download restriction value" error and allowing the download to proceed.

This PR will fix issue #2974.

PF4Public commented 2 months ago

So it is either allow downloads (by default), or block all of them?

berkley4 commented 2 months ago

So it is either allow downloads (by default), or block all of them?

That's correct.