[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

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));

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(); }
 

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

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_

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() {

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_;
 

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.

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

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;

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.

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))

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());

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());

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

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();
 }
 

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;
 

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_

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

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();
 };