# Disables file download quarantining --- a/components/download/internal/common/base_file.cc +++ b/components/download/internal/common/base_file.cc @@ -26,7 +26,6 @@ #include "components/download/public/common/download_interrupt_reasons_utils.h" #include "components/download/public/common/download_item.h" #include "components/download/public/common/download_stats.h" -#include "components/services/quarantine/quarantine.h" #include "crypto/hash.h" #include "crypto/secure_hash.h" @@ -538,94 +537,10 @@ DownloadInterruptReason BaseFile::Publis } #endif // BUILDFLAG(IS_ANDROID) -namespace { - -DownloadInterruptReason QuarantineFileResultToReason( - quarantine::mojom::QuarantineFileResult result) { - switch (result) { - case quarantine::mojom::QuarantineFileResult::OK: - return DOWNLOAD_INTERRUPT_REASON_NONE; - case quarantine::mojom::QuarantineFileResult::VIRUS_INFECTED: - return DOWNLOAD_INTERRUPT_REASON_FILE_VIRUS_INFECTED; - case quarantine::mojom::QuarantineFileResult::SECURITY_CHECK_FAILED: - return DOWNLOAD_INTERRUPT_REASON_FILE_SECURITY_CHECK_FAILED; - case quarantine::mojom::QuarantineFileResult::BLOCKED_BY_POLICY: - return DOWNLOAD_INTERRUPT_REASON_FILE_BLOCKED; - case quarantine::mojom::QuarantineFileResult::ACCESS_DENIED: - return DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED; - - case quarantine::mojom::QuarantineFileResult::FILE_MISSING: - // Don't have a good interrupt reason here. This return code means that - // the file at |full_path_| went missing before QuarantineFile got to - // look at it. Not expected to happen, but we've seen instances where a - // file goes missing immediately after BaseFile closes the handle. - // - // Intentionally using a different error message than - // SECURITY_CHECK_FAILED in order to distinguish the two. - return DOWNLOAD_INTERRUPT_REASON_FILE_FAILED; - - case quarantine::mojom::QuarantineFileResult::ANNOTATION_FAILED: - // This means that the mark-of-the-web couldn't be applied. The file is - // already on the file system under its final target name. - // - // Causes of failed annotations typically aren't transient. E.g. the - // target file system may not support extended attributes or alternate - // streams. We are going to allow these downloads to progress on the - // assumption that failures to apply MOTW can't reliably be introduced - // remotely. - return DOWNLOAD_INTERRUPT_REASON_NONE; - } - return DOWNLOAD_INTERRUPT_REASON_FILE_FAILED; -} - -} // namespace - // static GURL BaseFile::GetEffectiveAuthorityURL(const GURL& source_url, const GURL& referrer_url) { - if (source_url.is_valid()) { - // http{,s} has an authority and are supported. - if (source_url.SchemeIsHTTPOrHTTPS()) - return source_url; - - // If the download source is file:// ideally we should copy the MOTW from - // the original file, but given that Chrome/Chromium places strict - // restrictions on which schemes can reference file:// URLs, this code is - // going to assume that at this point it's okay to treat this download as - // being from the local system. - if (source_url.SchemeIsFile()) return source_url; - - // ftp:// has an authority. - if (source_url.SchemeIs(url::kFtpScheme)) - return source_url; - - if (source_url.SchemeIs(url::kBlobScheme)) - return url::Origin::Create(source_url).GetURL(); - } - - if (referrer_url.is_valid() && referrer_url.SchemeIsHTTPOrHTTPS()) - return referrer_url; - - return GURL(); -} - -void BaseFile::OnFileQuarantined( - quarantine::mojom::QuarantineFileResult result) { - DCHECK(on_annotation_done_callback_); - quarantine_service_.reset(); - std::move(on_annotation_done_callback_) - .Run(QuarantineFileResultToReason(result)); -} - -void BaseFile::OnQuarantineServiceError(const GURL& source_url, - const GURL& referrer_url) { -#if BUILDFLAG(IS_WIN) - OnFileQuarantined(quarantine::SetInternetZoneIdentifierDirectly( - full_path_, source_url, referrer_url)); -#else // !BUILDFLAG(IS_WIN) - NOTREACHED() << "In-process quarantine service should not have failed."; -#endif // !BUILDFLAG(IS_WIN) } void BaseFile::AnnotateWithSourceInformation( @@ -635,32 +550,8 @@ void BaseFile::AnnotateWithSourceInforma const std::optional& request_initiator, mojo::PendingRemote remote_quarantine, OnAnnotationDoneCallback on_annotation_done_callback) { - GURL authority_url = GetEffectiveAuthorityURL(source_url, referrer_url); - if (!remote_quarantine) { -#if BUILDFLAG(IS_WIN) - quarantine::mojom::QuarantineFileResult result = - quarantine::SetInternetZoneIdentifierDirectly(full_path_, authority_url, - referrer_url); -#else - quarantine::mojom::QuarantineFileResult result = - quarantine::mojom::QuarantineFileResult::ANNOTATION_FAILED; -#endif - std::move(on_annotation_done_callback) - .Run(QuarantineFileResultToReason(result)); - } else { - quarantine_service_.Bind(std::move(remote_quarantine)); - - on_annotation_done_callback_ = std::move(on_annotation_done_callback); - - quarantine_service_.set_disconnect_handler(base::BindOnce( - &BaseFile::OnQuarantineServiceError, weak_factory_.GetWeakPtr(), - authority_url, referrer_url)); - - quarantine_service_->QuarantineFile( - full_path_, authority_url, referrer_url, request_initiator, client_guid, - base::BindOnce(&BaseFile::OnFileQuarantined, - weak_factory_.GetWeakPtr())); - } + std::move(on_annotation_done_callback) + .Run(DOWNLOAD_INTERRUPT_REASON_NONE); } } // namespace download --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn @@ -139,7 +139,6 @@ source_set("browser") { "//components/persistent_cache", "//components/power_monitor", "//components/services/filesystem:lib", - "//components/services/quarantine", "//components/services/storage", "//components/services/storage:filesystem_proxy_factory", "//components/services/storage/dom_storage:local_storage_proto", --- a/content/browser/file_system_access/file_system_access_safe_move_helper.cc +++ b/content/browser/file_system_access/file_system_access_safe_move_helper.cc @@ -14,7 +14,6 @@ #include "base/task/thread_pool.h" #include "base/thread_annotations.h" #include "build/build_config.h" -#include "components/services/quarantine/quarantine.h" #include "content/browser/file_system_access/features.h" #include "content/browser/file_system_access/file_system_access_error.h" #include "content/public/browser/content_browser_client.h" @@ -231,22 +230,9 @@ void FileSystemAccessSafeMoveHelper::Did // not exist anymore. In case of error, the source file URL will point to a // valid filesystem location. base::OnceCallback result_callback; - if (RequireQuarantine()) { - GURL referrer_url = manager_->is_off_the_record() ? GURL() : context_.url; - mojo::Remote quarantine_remote; - if (quarantine_connection_callback_) { - quarantine_connection_callback_.Run( - quarantine_remote.BindNewPipeAndPassReceiver()); - } - result_callback = - base::BindOnce(&FileSystemAccessSafeMoveHelper::DidFileDoQuarantine, - weak_factory_.GetWeakPtr(), dest_url(), referrer_url, - std::move(quarantine_remote)); - } else { result_callback = base::BindOnce(&FileSystemAccessSafeMoveHelper::DidFileSkipQuarantine, weak_factory_.GetWeakPtr()); - } manager_->DoFileSystemOperation( FROM_HERE, &storage::FileSystemOperationRunner::Move, std::move(result_callback), source_url(), dest_url(), options_, @@ -263,7 +249,6 @@ void FileSystemAccessSafeMoveHelper::Did void FileSystemAccessSafeMoveHelper::DidFileDoQuarantine( const storage::FileSystemURL& target_url, const GURL& referrer_url, - mojo::Remote quarantine_remote, base::File::Error result) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -294,40 +279,9 @@ void FileSystemAccessSafeMoveHelper::Did referrer_url.is_valid() && referrer_url.SchemeIsHTTPOrHTTPS() ? referrer_url : GURL(); - - if (quarantine_remote) { - quarantine::mojom::Quarantine* raw_quarantine = quarantine_remote.get(); - raw_quarantine->QuarantineFile( - target_url.path(), authority_url, referrer_url, - // TODO(crbug.com/351165321): Consider propagating request_initiator - // information here. - /*request_initiator=*/std::nullopt, - GetContentClient() - ->browser() - ->GetApplicationClientGUIDForQuarantineCheck(), - mojo::WrapCallbackWithDefaultInvokeIfNotRun( - base::BindOnce(&FileSystemAccessSafeMoveHelper::DidAnnotateFile, - weak_factory_.GetWeakPtr(), - std::move(quarantine_remote)), - quarantine::mojom::QuarantineFileResult::ANNOTATION_FAILED)); - } else { -#if BUILDFLAG(IS_WIN) - base::ThreadPool::PostTaskAndReplyWithResult( - FROM_HERE, {base::MayBlock()}, - base::BindOnce(&quarantine::SetInternetZoneIdentifierDirectly, - target_url.path(), authority_url, referrer_url), - base::BindOnce(&FileSystemAccessSafeMoveHelper::DidAnnotateFile, - weak_factory_.GetWeakPtr(), - std::move(quarantine_remote))); -#else - DidAnnotateFile(std::move(quarantine_remote), - quarantine::mojom::QuarantineFileResult::ANNOTATION_FAILED); -#endif - } } void FileSystemAccessSafeMoveHelper::DidAnnotateFile( - mojo::Remote quarantine_remote, quarantine::mojom::QuarantineFileResult result) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); --- a/content/browser/file_system_access/file_system_access_safe_move_helper.h +++ b/content/browser/file_system_access/file_system_access_safe_move_helper.h @@ -66,10 +66,8 @@ class CONTENT_EXPORT FileSystemAccessSaf void DidFileDoQuarantine( const storage::FileSystemURL& target_url, const GURL& referrer_url, - mojo::Remote quarantine_remote, base::File::Error result); void DidAnnotateFile( - mojo::Remote quarantine_remote, quarantine::mojom::QuarantineFileResult result); void ComputeHashForSourceFile(HashCallback callback);