summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorgbillock@chromium.org <gbillock@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-08-10 13:49:48 +0000
committergbillock@chromium.org <gbillock@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-08-10 13:49:48 +0000
commit492c00974a1354e97e27939279eecd7fc9d8d9ce (patch)
tree899e3da8b00bb60209a012a0587564b50477c017
parent6cf51b6c67396f11d6e4c2cccb75e8ed3a020e37 (diff)
downloadchromium_src-492c00974a1354e97e27939279eecd7fc9d8d9ce.zip
chromium_src-492c00974a1354e97e27939279eecd7fc9d8d9ce.tar.gz
chromium_src-492c00974a1354e97e27939279eecd7fc9d8d9ce.tar.bz2
[Downloads] Move client guid for AV scanning of downloaded files to chrome/
This client app should not be used by all clients of content/. Moving to chrome so it can be managed there and re-used if needed. Review URL: https://chromiumcodereview.appspot.com/21355004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@216843 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/download/chrome_download_manager_delegate.cc6
-rw-r--r--chrome/browser/download/chrome_download_manager_delegate.h1
-rw-r--r--chrome/common/chrome_constants.cc6
-rw-r--r--chrome/common/chrome_constants.h3
-rw-r--r--content/browser/download/base_file.cc4
-rw-r--r--content/browser/download/base_file.h9
-rw-r--r--content/browser/download/base_file_win.cc14
-rw-r--r--content/browser/download/download_file.h5
-rw-r--r--content/browser/download/download_file_impl.cc4
-rw-r--r--content/browser/download/download_file_impl.h1
-rw-r--r--content/browser/download/download_file_unittest.cc2
-rw-r--r--content/browser/download/download_manager_impl.cc7
-rw-r--r--content/browser/download/download_manager_impl_unittest.cc11
-rw-r--r--content/browser/download/mock_download_file.h1
-rw-r--r--content/browser/download/save_file.cc2
-rw-r--r--content/browser/safe_util_win.cc88
-rw-r--r--content/browser/safe_util_win.h41
-rw-r--r--content/public/browser/download_manager_delegate.cc5
-rw-r--r--content/public/browser/download_manager_delegate.h6
19 files changed, 106 insertions, 110 deletions
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index a810537..aec10bb 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -34,6 +34,7 @@
#include "chrome/browser/platform_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/common/chrome_constants.h"
#include "chrome/common/pref_names.h"
#include "components/user_prefs/pref_registry_syncable.h"
#include "content/public/browser/download_item.h"
@@ -402,6 +403,11 @@ void ChromeDownloadManagerDelegate::CheckForFileExistence(
callback);
}
+std::string
+ChromeDownloadManagerDelegate::ApplicationClientIdForFileScanning() const {
+ return std::string(chrome::kApplicationClientIDStringForAVScanning);
+}
+
DownloadProtectionService*
ChromeDownloadManagerDelegate::GetDownloadProtectionService() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h
index e2c13ad..d23337d 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.h
+++ b/chrome/browser/download/chrome_download_manager_delegate.h
@@ -93,6 +93,7 @@ class ChromeDownloadManagerDelegate
virtual void CheckForFileExistence(
content::DownloadItem* download,
const content::CheckForFileExistenceCallback& callback) OVERRIDE;
+ virtual std::string ApplicationClientIdForFileScanning() const OVERRIDE;
DownloadPrefs* download_prefs() { return download_prefs_.get(); }
diff --git a/chrome/common/chrome_constants.cc b/chrome/common/chrome_constants.cc
index 874a039..40621f8 100644
--- a/chrome/common/chrome_constants.cc
+++ b/chrome/common/chrome_constants.cc
@@ -253,6 +253,12 @@ const wchar_t kLaunchModeValue[] = L"launch_mode";
const char kProfileDirPrefix[] = "u-";
#endif
+// This GUID is associated with any 'don't ask me again' settings that the
+// user can select for different file types.
+// {2676A9A2-D919-4FEE-9187-152100393AB2}
+const char kApplicationClientIDStringForAVScanning[] =
+ "2676A9A2-D919-4FEE-9187-152100393AB2";
+
} // namespace chrome
#undef FPL
diff --git a/chrome/common/chrome_constants.h b/chrome/common/chrome_constants.h
index 6775c80..cd97f7c 100644
--- a/chrome/common/chrome_constants.h
+++ b/chrome/common/chrome_constants.h
@@ -139,6 +139,9 @@ extern const wchar_t kLaunchModeValue[];
extern const char kProfileDirPrefix[];
#endif
+// Used to identify the application to the system AV function in Windows.
+extern const char kApplicationClientIDStringForAVScanning[];
+
} // namespace chrome
#endif // CHROME_COMMON_CHROME_CONSTANTS_H_
diff --git a/content/browser/download/base_file.cc b/content/browser/download/base_file.cc
index 466be32..90ab485 100644
--- a/content/browser/download/base_file.cc
+++ b/content/browser/download/base_file.cc
@@ -211,6 +211,10 @@ void BaseFile::Finish() {
Close();
}
+void BaseFile::SetClientGuid(const std::string& guid) {
+ client_guid_ = guid;
+}
+
// OS_WIN, OS_MACOSX and OS_LINUX have specialized implementations.
#if !defined(OS_WIN) && !defined(OS_MACOSX) && !defined(OS_LINUX)
DownloadInterruptReason BaseFile::AnnotateWithSourceInformation() {
diff --git a/content/browser/download/base_file.h b/content/browser/download/base_file.h
index 9c1261c..f1686c8 100644
--- a/content/browser/download/base_file.h
+++ b/content/browser/download/base_file.h
@@ -69,8 +69,15 @@ class CONTENT_EXPORT BaseFile {
// Indicate that the download has finished. No new data will be received.
void Finish();
+ // Set the client guid which will be used to identify the app to the
+ // system AV scanning function. Should be called before
+ // AnnotateWithSourceInformation() to take effect.
+ void SetClientGuid(const std::string& guid);
+
// Informs the OS that this file came from the internet. Returns a
// DownloadInterruptReason indicating the result of the operation.
+ // Note: SetClientGuid() should be called before this function on
+ // Windows to ensure the correct app client ID is available.
DownloadInterruptReason AnnotateWithSourceInformation();
base::FilePath full_path() const { return full_path_; }
@@ -142,6 +149,8 @@ class CONTENT_EXPORT BaseFile {
// The URL where the download was initiated.
GURL referrer_url_;
+ std::string client_guid_;
+
// OS file stream for writing
scoped_ptr<net::FileStream> file_stream_;
diff --git a/content/browser/download/base_file_win.cc b/content/browser/download/base_file_win.cc
index 398e5fc..252f0495 100644
--- a/content/browser/download/base_file_win.cc
+++ b/content/browser/download/base_file_win.cc
@@ -5,9 +5,12 @@
#include "content/browser/download/base_file.h"
#include <windows.h>
+#include <cguid.h>
+#include <objbase.h>
#include <shellapi.h>
#include "base/file_util.h"
+#include "base/guid.h"
#include "base/metrics/histogram.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_restrictions.h"
@@ -326,7 +329,16 @@ DownloadInterruptReason BaseFile::AnnotateWithSourceInformation() {
bound_net_log_.BeginEvent(net::NetLog::TYPE_DOWNLOAD_FILE_ANNOTATED);
DownloadInterruptReason result = DOWNLOAD_INTERRUPT_REASON_NONE;
- HRESULT hr = ScanAndSaveDownloadedFile(full_path_, source_url_);
+ std::string braces_guid = "{" + client_guid_ + "}";
+ GUID guid = GUID_NULL;
+ if (base::IsValidGUID(client_guid_)) {
+ HRESULT hr = CLSIDFromString(
+ base::UTF8ToUTF16(braces_guid).c_str(), &guid);
+ if (FAILED(hr))
+ guid = GUID_NULL;
+ }
+
+ HRESULT hr = AVScanFile(full_path_, source_url_.spec(), guid);
// If the download file is missing after the call, then treat this as an
// interrupted download.
diff --git a/content/browser/download/download_file.h b/content/browser/download/download_file.h
index a8e18ff..568e7ee 100644
--- a/content/browser/download/download_file.h
+++ b/content/browser/download/download_file.h
@@ -73,6 +73,11 @@ class CONTENT_EXPORT DownloadFile {
// Returns the current (intermediate) state of the hash as a byte string.
virtual std::string GetHashState() = 0;
+ // Set the application GUID to be used to identify the app to the
+ // system AV function when scanning downloaded files. Should be called
+ // before RenameAndAnnotate() to take effect.
+ virtual void SetClientGuid(const std::string& guid) = 0;
+
// For testing. Must be called on FILE thread.
// TODO(rdsmith): Replace use of EnsureNoPendingDownloads()
// on the DownloadManager with a test-specific DownloadFileFactory
diff --git a/content/browser/download/download_file_impl.cc b/content/browser/download/download_file_impl.cc
index bbe317b..67bee81 100644
--- a/content/browser/download/download_file_impl.cc
+++ b/content/browser/download/download_file_impl.cc
@@ -200,6 +200,10 @@ std::string DownloadFileImpl::GetHashState() {
return file_.GetHashState();
}
+void DownloadFileImpl::SetClientGuid(const std::string& guid) {
+ file_.SetClientGuid(guid);
+}
+
void DownloadFileImpl::StreamActive() {
base::TimeTicks start(base::TimeTicks::Now());
base::TimeTicks now;
diff --git a/content/browser/download/download_file_impl.h b/content/browser/download/download_file_impl.h
index 963ae9f..9a50870 100644
--- a/content/browser/download/download_file_impl.h
+++ b/content/browser/download/download_file_impl.h
@@ -63,6 +63,7 @@ class CONTENT_EXPORT DownloadFileImpl : virtual public DownloadFile {
virtual int64 CurrentSpeed() const OVERRIDE;
virtual bool GetHash(std::string* hash) OVERRIDE;
virtual std::string GetHashState() OVERRIDE;
+ virtual void SetClientGuid(const std::string& guid) OVERRIDE;
protected:
// For test class overrides.
diff --git a/content/browser/download/download_file_unittest.cc b/content/browser/download/download_file_unittest.cc
index ebf501d..867b76f 100644
--- a/content/browser/download/download_file_unittest.cc
+++ b/content/browser/download/download_file_unittest.cc
@@ -138,6 +138,8 @@ class DownloadFileTest : public testing::Test {
net::BoundNetLog(),
scoped_ptr<PowerSaveBlocker>().Pass(),
observer_factory_.GetWeakPtr()));
+ download_file_->SetClientGuid(
+ "12345678-ABCD-1234-DCBA-123456789ABC");
EXPECT_CALL(*input_stream_, Read(_, _))
.WillOnce(Return(ByteStreamReader::STREAM_EMPTY))
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index f9241832..ccfb4cc 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -422,6 +422,13 @@ void DownloadManagerImpl::StartDownloadWithId(
delegate_->GenerateFileHash(),
stream.Pass(), download->GetBoundNetLog(),
download->DestinationObserverAsWeakPtr()));
+
+ // Attach the client ID identifying the app to the AV system.
+ if (download_file.get() && delegate_) {
+ download_file->SetClientGuid(
+ delegate_->ApplicationClientIdForFileScanning());
+ }
+
scoped_ptr<DownloadRequestHandleInterface> req_handle(
new DownloadRequestHandle(info->request_handle));
download->Start(download_file.Pass(), req_handle.Pass());
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index a8e18d4..eba2833 100644
--- a/content/browser/download/download_manager_impl_unittest.cc
+++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -195,6 +195,7 @@ class MockDownloadManagerDelegate : public DownloadManagerDelegate {
MOCK_METHOD5(ChooseSavePath, void(
WebContents*, const base::FilePath&, const base::FilePath::StringType&,
bool, const SavePackagePathPickedCallback&));
+ MOCK_CONST_METHOD0(ApplicationClientIdForFileScanning, std::string());
};
MockDownloadManagerDelegate::MockDownloadManagerDelegate() {}
@@ -363,7 +364,7 @@ class MockDownloadFileFactory
virtual ~MockDownloadFileFactory() {}
// Overridden method from DownloadFileFactory
- MOCK_METHOD8(MockCreateFile, DownloadFile*(
+ MOCK_METHOD8(MockCreateFile, MockDownloadFile*(
const DownloadSaveInfo&,
const base::FilePath&,
const GURL&, const GURL&, bool,
@@ -593,9 +594,15 @@ TEST_F(DownloadManagerTest, StartDownload) {
EXPECT_CALL(GetMockDownloadManagerDelegate(), GetSaveDir(_, _, _, _));
EXPECT_CALL(GetMockDownloadManagerDelegate(), GenerateFileHash())
.WillOnce(Return(true));
+ EXPECT_CALL(GetMockDownloadManagerDelegate(),
+ ApplicationClientIdForFileScanning())
+ .WillRepeatedly(Return("client-id"));
+ MockDownloadFile* mock_file = new MockDownloadFile;
+ EXPECT_CALL(*mock_file, SetClientGuid("client-id"));
EXPECT_CALL(*mock_download_file_factory_.get(),
MockCreateFile(Ref(*info->save_info.get()), _, _, _, true,
- stream.get(), _, _));
+ stream.get(), _, _))
+ .WillOnce(Return(mock_file));
download_manager_->StartDownload(
info.Pass(), stream.Pass(), DownloadUrlParameters::OnStartedCallback());
diff --git a/content/browser/download/mock_download_file.h b/content/browser/download/mock_download_file.h
index 74dc75c..5d4b270 100644
--- a/content/browser/download/mock_download_file.h
+++ b/content/browser/download/mock_download_file.h
@@ -48,6 +48,7 @@ class MockDownloadFile : virtual public DownloadFile {
MOCK_CONST_METHOD0(Id, int());
MOCK_METHOD0(GetDownloadManager, DownloadManager*());
MOCK_CONST_METHOD0(DebugString, std::string());
+ MOCK_METHOD1(SetClientGuid, void(const std::string&));
};
} // namespace content
diff --git a/content/browser/download/save_file.cc b/content/browser/download/save_file.cc
index 80caf13..e8885e4 100644
--- a/content/browser/download/save_file.cc
+++ b/content/browser/download/save_file.cc
@@ -60,6 +60,8 @@ void SaveFile::Finish() {
}
void SaveFile::AnnotateWithSourceInformation() {
+ // TODO(gbillock): If this method is called, it should set the
+ // file_.SetClientGuid() method first.
file_.AnnotateWithSourceInformation();
}
diff --git a/content/browser/safe_util_win.cc b/content/browser/safe_util_win.cc
index 2dce2ca..ac077f1 100644
--- a/content/browser/safe_util_win.cc
+++ b/content/browser/safe_util_win.cc
@@ -19,12 +19,6 @@
namespace content {
namespace {
-// This GUID is associated with any 'don't ask me again' settings that the
-// user can select for different file types.
-// {2676A9A2-D919-4fee-9187-152100393AB2}
-static const GUID kClientID = { 0x2676a9a2, 0xd919, 0x4fee,
- { 0x91, 0x87, 0x15, 0x21, 0x0, 0x39, 0x3a, 0xb2 } };
-
// Sets the Zone Identifier on the file to "Internet" (3). Returns true if the
// function succeeds, false otherwise. A failure is expected on system where
// the Zone Identifier is not supported, like a machine with a FAT32 filesystem.
@@ -55,72 +49,11 @@ bool SetInternetZoneIdentifierDirectly(const base::FilePath& full_path) {
return true;
}
-}
-
-// This function implementation is based on the attachment execution
-// services functionally deployed with IE6 or Service pack 2. This
-// functionality is exposed in the IAttachmentExecute COM interface.
-// more information at:
-// http://msdn2.microsoft.com/en-us/library/ms647048.aspx
-bool SaferOpenItemViaShell(HWND hwnd, const std::wstring& window_title,
- const base::FilePath& full_path,
- const std::wstring& source_url) {
- base::win::ScopedComPtr<IAttachmentExecute> attachment_services;
- HRESULT hr = attachment_services.CreateInstance(CLSID_AttachmentServices);
- if (FAILED(hr)) {
- // We don't have Attachment Execution Services, it must be a pre-XP.SP2
- // Windows installation, or the thread does not have COM initialized.
- if (hr == CO_E_NOTINITIALIZED) {
- NOTREACHED();
- return false;
- }
- return ui::win::OpenItemViaShell(full_path);
- }
-
- attachment_services->SetClientGuid(kClientID);
-
- if (!window_title.empty())
- attachment_services->SetClientTitle(window_title.c_str());
-
- // To help windows decide if the downloaded file is dangerous we can provide
- // what the documentation calls evidence. Which we provide now:
- //
- // Set the file itself as evidence.
- hr = attachment_services->SetLocalPath(full_path.value().c_str());
- if (FAILED(hr))
- return false;
- // Set the origin URL as evidence.
- hr = attachment_services->SetSource(source_url.c_str());
- if (FAILED(hr))
- return false;
+} // namespace
- // Now check the windows policy.
- if (attachment_services->CheckPolicy() != S_OK) {
- // It is possible that the above call returns an undocumented result
- // equal to 0x800c000e which seems to indicate that the URL failed the
- // the security check. If you proceed with the Prompt() call the
- // Shell might show a dialog that says:
- // "windows found that this file is potentially harmful. To help protect
- // your computer, Windows has blocked access to this file."
- // Upon dismissal of the dialog windows will delete the file (!!).
- // So, we can 'return' in that case but maybe is best to let it happen to
- // fail on the safe side.
-
- ATTACHMENT_ACTION action;
- // We cannot control what the prompt says or does directly but it
- // is a pretty decent dialog; for example, if an executable is signed it can
- // decode and show the publisher and the certificate.
- hr = attachment_services->Prompt(hwnd, ATTACHMENT_PROMPT_EXEC, &action);
- if (FAILED(hr) || (ATTACHMENT_ACTION_CANCEL == action)) {
- // The user has declined opening the item.
- return false;
- }
- }
- return ui::win::OpenItemViaShellNoZoneCheck(full_path);
-}
-
-HRESULT ScanAndSaveDownloadedFile(const base::FilePath& full_path,
- const GURL& source_url) {
+HRESULT AVScanFile(const base::FilePath& full_path,
+ const std::string& source_url,
+ const GUID& client_guid) {
base::win::ScopedComPtr<IAttachmentExecute> attachment_services;
HRESULT hr = attachment_services.CreateInstance(CLSID_AttachmentServices);
@@ -135,15 +68,20 @@ HRESULT ScanAndSaveDownloadedFile(const base::FilePath& full_path,
return hr;
}
- hr = attachment_services->SetClientGuid(kClientID);
- if (FAILED(hr))
- return hr;
+ if (!IsEqualGUID(client_guid, GUID_NULL)) {
+ hr = attachment_services->SetClientGuid(client_guid);
+ if (FAILED(hr))
+ return hr;
+ }
hr = attachment_services->SetLocalPath(full_path.value().c_str());
if (FAILED(hr))
return hr;
- hr = attachment_services->SetSource(UTF8ToWide(source_url.spec()).c_str());
+ // Note: SetSource looks like it needs to be called, even if empty.
+ // Docs say it is optional, but it appears not calling it at all sets
+ // a zone that is too restrictive.
+ hr = attachment_services->SetSource(UTF8ToWide(source_url).c_str());
if (FAILED(hr))
return hr;
diff --git a/content/browser/safe_util_win.h b/content/browser/safe_util_win.h
index 1636748..3860429 100644
--- a/content/browser/safe_util_win.h
+++ b/content/browser/safe_util_win.h
@@ -16,34 +16,6 @@ class FilePath;
namespace content {
-// Open or run a downloaded file via the Windows shell, possibly showing first
-// a consent dialog if the the file is deemed dangerous. This function is an
-// enhancement over the OpenItemViaShell() function of win_util.h.
-//
-// The user consent dialog will be shown or not according to the windows
-// execution policy defined in the registry which can be overridden per user.
-// The mechanics of the policy are explained in the Microsoft Knowledge base
-// number 883260: http://support.microsoft.com/kb/883260
-//
-// The 'hwnd' is the handle to the parent window. In case a dialog is displayed
-// the parent window will be disabled since the dialog is meant to be modal.
-// The 'window_title' is the text displayed on the title bar of the dialog. If
-// you pass an empty string the dialog will have a generic 'windows security'
-// name on the title bar.
-//
-// You must provide a valid 'full_path' to the file to be opened and a well
-// formed url in 'source_url'. The url should identify the source of the file
-// but does not have to be network-reachable. If the url is malformed a
-// dialog will be shown telling the user that the file will be blocked.
-//
-// In the event that there is no default application registered for the file
-// specified by 'full_path' it ask the user, via the Windows "Open With"
-// dialog.
-// Returns 'true' on successful open, 'false' otherwise.
-bool SaferOpenItemViaShell(HWND hwnd, const std::wstring& window_title,
- const base::FilePath& full_path,
- const std::wstring& source_url);
-
// Invokes IAttachmentExecute::Save to validate the downloaded file. The call
// may scan the file for viruses and if necessary, annotate it with evidence. As
// a result of the validation, the file may be deleted. See:
@@ -67,10 +39,15 @@ bool SaferOpenItemViaShell(HWND hwnd, const std::wstring& window_title,
// Any other return value indicates an unexpected error during the scan.
//
// |full_path| : is the path to the downloaded file. This should be the final
-// path of the download.
-// |source_url|: the source URL for the download.
-HRESULT ScanAndSaveDownloadedFile(const base::FilePath& full_path,
- const GURL& source_url);
+// path of the download. Must be present.
+// |source_url|: the source URL for the download. If empty, the source will
+// not be set.
+// |client_guid|: the GUID to be set in the IAttachmentExecute client slot.
+// Used to identify the app to the system AV function.
+// If GUID_NULL is passed, no client GUID is set.
+HRESULT AVScanFile(const base::FilePath& full_path,
+ const std::string& source_url,
+ const GUID& client_guid);
} // namespace content
#endif // CONTENT_COMMON_SAFE_UTIL_WIN_H_
diff --git a/content/public/browser/download_manager_delegate.cc b/content/public/browser/download_manager_delegate.cc
index c33fcd4..9351ce2 100644
--- a/content/public/browser/download_manager_delegate.cc
+++ b/content/public/browser/download_manager_delegate.cc
@@ -38,6 +38,11 @@ bool DownloadManagerDelegate::GenerateFileHash() {
return false;
}
+std::string
+DownloadManagerDelegate::ApplicationClientIdForFileScanning() const {
+ return std::string();
+}
+
DownloadManagerDelegate::~DownloadManagerDelegate() {}
} // namespace content
diff --git a/content/public/browser/download_manager_delegate.h b/content/public/browser/download_manager_delegate.h
index 98bad43..89be2df 100644
--- a/content/public/browser/download_manager_delegate.h
+++ b/content/public/browser/download_manager_delegate.h
@@ -129,6 +129,12 @@ class CONTENT_EXPORT DownloadManagerDelegate {
DownloadItem* download,
const CheckForFileExistenceCallback& callback) {}
+ // Return a GUID string used for identifying the application to the
+ // system AV function for scanning downloaded files. If an empty
+ // or invalid GUID string is returned, no client identification
+ // will be given to the AV function.
+ virtual std::string ApplicationClientIdForFileScanning() const;
+
protected:
virtual ~DownloadManagerDelegate();
};