Add payload Authenticode verification build flag

omaha/base/constants.h

@@ -281,9 +281,10 @@ const TCHAR* const kRegValueAutoUpdateJitterMs      = _T("AutoUpdateJitterMs");
 const TCHAR* const kRegValueProxyHost               = _T("ProxyHost");
 const TCHAR* const kRegValueProxyPort               = _T("ProxyPort");
 const TCHAR* const kRegValueMID                     = _T("mid");
+
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
 const TCHAR* const kRegValueDisablePayloadAuthenticodeVerification =
     _T("DisablePayloadAuthenticodeVerification");
-
 // Update binaries with the following extensions are Authenticode-verified
 // unless DisablePayloadAuthenticodeVerification above is non-zero in registry.
 const TCHAR* const kAuthenticodeSignedExtensions[] = {
@@ -292,6 +293,7 @@ const TCHAR* const kAuthenticodeSignedExtensions[] = {
   _T("psd1"), _T("ps1xml"), _T("psc1"), _T("acm "), _T("ax"),  _T("cpl"),
   _T("drv"),  _T("efi"),    _T("mui"),  _T("scr"),  _T("sys"), _T("tsp")
 };
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
 
 #if defined(HAS_DEVICE_MANAGEMENT)
 const TCHAR* const kRegValueNameDeviceManagementUrl = _T("DeviceManagementUrl");

omaha/common/config_manager.cc

@@ -1807,6 +1807,8 @@ bool ConfigManager::AlwaysAllowCrashUploads() const {
   return always_allow_crash_uploads != 0;
 }
 
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+
 bool ConfigManager::ShouldVerifyPayloadAuthenticodeSignature() const {
   DWORD disabled_in_registry = 0;
   RegKey::GetValue(MACHINE_REG_UPDATE_DEV,
@@ -1815,6 +1817,8 @@ bool ConfigManager::ShouldVerifyPayloadAuthenticodeSignature() const {
   return disabled_in_registry == 0;
 }
 
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+
 int ConfigManager::MaxCrashUploadsPerDay() const {
   DWORD num_uploads = 0;
   if (FAILED(RegKey::GetValue(MACHINE_REG_UPDATE_DEV,

omaha/common/config_manager.h

@@ -460,9 +460,11 @@ class ConfigManager {
   // build flavor or other configuration parameters.
   bool AlwaysAllowCrashUploads() const;
 
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
   // Returns whether the Authenticode signature of update payloads should be
   // verified.
   bool ShouldVerifyPayloadAuthenticodeSignature() const;
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
 
   // Returns the number of crashes to upload per day.
   int MaxCrashUploadsPerDay() const;

omaha/goopdate/download_manager.cc

@@ -41,7 +41,6 @@
 #include "omaha/base/utils.h"
 #include "omaha/common/config_manager.h"
 #include "omaha/common/const_goopdate.h"
-#include "omaha/common/google_signaturevalidator.h"
 #include "omaha/goopdate/model.h"
 #include "omaha/goopdate/package_cache.h"
 #include "omaha/goopdate/server_resource.h"
@@ -54,6 +53,10 @@
 #include "omaha/net/net_utils.h"
 #include "omaha/net/simple_request.h"
 
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+#include "omaha/common/google_signaturevalidator.h"
+#endif
+
 namespace omaha {
 
 namespace {
@@ -477,11 +480,20 @@ HRESULT DownloadManager::DoDownloadPackageFromUrl(const CString& url,
 
   // We copy the file to the Package Cache unimpersonated, since the package
   // cache is in a privileged location.
-  hr = CallAsSelfAndImpersonate3(this,
-                                 &DownloadManager::CachePackage,
-                                 static_cast<const Package*>(package),
-                                 &source_file,
-                                 &filename);
+  hr =
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+  CallAsSelfAndImpersonate3
+#else
+  CallAsSelfAndImpersonate2
+#endif
+    (this,
+     &DownloadManager::CachePackage,
+     static_cast<const Package*>(package),
+     &source_file
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+     , &filename
+#endif
+     );
   if (FAILED(hr)) {
     OPT_LOG(LE, (_T("[DownloadManager::CachePackage failed][%#x]"), hr));
   }
@@ -524,8 +536,11 @@ HRESULT DownloadManager::PurgeAppLowerVersions(const CString& app_id,
 }
 
 HRESULT DownloadManager::CachePackage(const Package* package,
-                                      File* source_file,
-                                      const CString* source_file_path) {
+                                      File* source_file
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+                                      , const CString* source_file_path
+#endif
+                                      ) {
   ASSERT1(package);
   ASSERT1(source_file);
 
@@ -535,6 +550,8 @@ HRESULT DownloadManager::CachePackage(const Package* package,
   PackageCache::Key key(app_id, version, package_name);
 
   HRESULT hr = E_UNEXPECTED;
+
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
   if (ConfigManager::Instance()->ShouldVerifyPayloadAuthenticodeSignature()) {
     hr = EnsureSignatureIsValid(*source_file_path);
     if (FAILED(hr)) {
@@ -543,6 +560,7 @@ HRESULT DownloadManager::CachePackage(const Package* package,
       return GOOPDATEDOWNLOAD_E_AUTHENTICODE_VERIFICATION_FAILED;
     }
   }
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
 
   hr = package_cache()->Put(
       key, source_file, package->expected_hash());
@@ -566,6 +584,8 @@ HRESULT DownloadManager::CachePackage(const Package* package,
   return hr;
 }
 
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+
 HRESULT DownloadManager::EnsureSignatureIsValid(const CString& file_path) {
   const TCHAR* ext = ::PathFindExtension(file_path);
   ASSERT1(ext);
@@ -580,6 +600,8 @@ HRESULT DownloadManager::EnsureSignatureIsValid(const CString& file_path) {
   return S_OK;
 }
 
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+
 // The file is initially downloaded to a temporary unique name, to account
 // for the case where the same file is downloaded by multiple callers.
 HRESULT DownloadManager::BuildUniqueFileName(const CString& filename,

omaha/goopdate/download_manager.h

@@ -42,8 +42,11 @@ class DownloadManagerInterface {
   virtual HRESULT PurgeAppLowerVersions(const CString& app_id,
                                         const CString& version) = 0;
   virtual HRESULT CachePackage(const Package* package,
-                               File* source_file,
-                               const CString* source_file_path) = 0;
+                               File* source_file
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+                               , const CString* source_file_path
+#endif
+                               ) = 0;
   virtual HRESULT DownloadApp(App* app) = 0;
   virtual HRESULT GetPackage(const Package* package,
                              const CString& dir) const = 0;
@@ -64,8 +67,11 @@ class DownloadManager : public DownloadManagerInterface {
                                         const CString& version);
 
   virtual HRESULT CachePackage(const Package* package,
-                               File* source_file,
-                               const CString* source_file_path);
+                               File* source_file
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+                               , const CString* source_file_path
+#endif
+                               );
 
   // Downloads the specified app and stores its packages in the package cache.
   //
@@ -130,7 +136,10 @@ class DownloadManager : public DownloadManagerInterface {
                                    const CString& filename,
                                    Package* package,
                                    State* state);
+
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
   HRESULT EnsureSignatureIsValid(const CString& file_path);
+#endif
 
   bool is_machine() const;
 

omaha/goopdate/download_manager_unittest.cc

@@ -16,7 +16,6 @@
 // TODO(omaha): why so many dependencies for this unit test?
 
 #include <atlstr.h>
-#include <vector>
 #include <windows.h>
 
 #include "omaha/base/app_util.h"
@@ -38,6 +37,10 @@
 #include "omaha/testing/unit_test.h"
 #include "omaha/third_party/smartany/scoped_any.h"
 
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+#include <vector>
+#endif
+
 using ::testing::_;
 using ::testing::Return;
 
@@ -88,6 +91,8 @@ class DownloadManagerTest : public AppTestBase {
     AppTestBase::SetUp();
 
     CleanupFiles();
+
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
     RegKey::GetValue(MACHINE_REG_UPDATE_DEV,
                      kRegValueDisablePayloadAuthenticodeVerification,
                      &disable_payload_authenticode_verification_);
@@ -97,6 +102,7 @@ class DownloadManagerTest : public AppTestBase {
           MACHINE_REG_UPDATE_DEV,
           kRegValueDisablePayloadAuthenticodeVerification));
     }
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
 
     download_manager_.reset(new DownloadManager(is_machine_));
     EXPECT_SUCCEEDED(download_manager_->Initialize());
@@ -105,6 +111,8 @@ class DownloadManagerTest : public AppTestBase {
   virtual void TearDown() {
     download_manager_.reset();
     CleanupFiles();
+
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
     if (disable_payload_authenticode_verification_) {
       EXPECT_SUCCEEDED(RegKey::SetValue(
           MACHINE_REG_UPDATE_DEV,
@@ -115,12 +123,14 @@ class DownloadManagerTest : public AppTestBase {
           MACHINE_REG_UPDATE_DEV,
           kRegValueDisablePayloadAuthenticodeVerification));
     }
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
 
     AppTestBase::TearDown();
   }
 
   virtual void CleanupFiles() = 0;
 
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
   void TestCachePackage(App* app,
                         const TCHAR* unittest_support_file_name,
                         HRESULT expected_result) {
@@ -153,6 +163,7 @@ class DownloadManagerTest : public AppTestBase {
     hr = download_manager_->CachePackage(package, &file, &file_path);
     EXPECT_EQ(expected_result, hr) << unittest_support_file_name;
   }
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
 
   static void SetAppStateCheckingForUpdate(App* app) {
     SetAppStateForUnitTest(app, new fsm::AppStateCheckingForUpdate);
@@ -164,7 +175,9 @@ class DownloadManagerTest : public AppTestBase {
 
   const CString cache_path_;
   std::unique_ptr<DownloadManager> download_manager_;
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
   DWORD disable_payload_authenticode_verification_ = 0; // Saved from registry
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
 };
 
 
@@ -1231,6 +1244,8 @@ TEST_F(DownloadManagerUserTest, GetPackage) {
   EXPECT_SUCCEEDED(DeleteDirectory(dir));
 }
 
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+
 TEST_F(DownloadManagerUserTest, CachePackage) {
   App* app = NULL;
   ASSERT_SUCCEEDED(app_bundle_->createApp(CComBSTR(kAppGuid1), &app));
@@ -1253,6 +1268,8 @@ TEST_F(DownloadManagerUserTest, CachePackage) {
   TestCachePackage(app, kFileWithOldCertificate, S_OK);
 }
 
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+
 TEST_F(DownloadManagerUserTest, GetPackage_NotPresent) {
   App* app = NULL;
   ASSERT_SUCCEEDED(app_bundle_->createApp(CComBSTR(kAppGuid1), &app));

omaha/goopdate/worker.cc

@@ -873,8 +873,11 @@ HRESULT Worker::CacheOfflinePackages(AppBundle* app_bundle) {
       }
 
       hr = download_manager_->CachePackage(package,
-                                           &offline_package_file,
-                                           &offline_package_path);
+                                           &offline_package_file
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
+                                           , &offline_package_path
+#endif
+                                           );
       if (FAILED(hr)) {
         CORE_LOG(LE, (_T("[CachePackage failed][%s][%s][0x%x][%Iu]"),
                       app->app_guid_string(), offline_package_path, hr, j));

omaha/goopdate/worker_unittest.cc

@@ -157,8 +157,13 @@ class MockDownloadManager : public DownloadManagerInterface {
       HRESULT());
   MOCK_METHOD2(PurgeAppLowerVersions,
       HRESULT(const CString&, const CString&));
+#ifdef VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
   MOCK_METHOD3(CachePackage,
       HRESULT(const Package*, File*, const CString*));
+#else
+  MOCK_METHOD2(CachePackage,
+               HRESULT(const Package*, File*));
+#endif // VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE
   MOCK_METHOD1(DownloadApp,
       HRESULT(App* app));
   MOCK_METHOD1(DownloadPackage,

omaha/main.scons

@@ -286,6 +286,8 @@ DeclareBit('analysis', 'Turns on static analysis')
 DeclareBit('suppress_light_validation', 'Suppress WiX Linker ICE validation')
 DeclareBit('has_device_management',
            'True if including cloud-based device management')
+DeclareBit('verify_payload_authenticode_signature',
+           'Whether to verify the Authenticode signature of payloads')
 
 # Build official installers if --official_installers is on the command line.
 win_env.SetBitFromOption('official_installers', False)
@@ -365,6 +367,9 @@ win_env.SetBitFromOption('analysis', False)
 # non-interactive account or a non-Admin account: http://goo.gl/UyDzeP.
 win_env.SetBitFromOption('suppress_light_validation', False)
 
+# Whether the Authenticode signature of update payloads should be verified.
+win_env.SetBitFromOption('verify_payload_authenticode_signature', False)
+
 #
 # Set up version info.
 #
@@ -413,6 +418,11 @@ if win_env.Bit('has_device_management'):
             CPPDEFINES = ['HAS_LEGACY_DM_CLIENT'],
         )
 
+if win_env.Bit('verify_payload_authenticode_signature'):
+    win_env.Append(
+        CPPDEFINES = ['VERIFY_PAYLOAD_AUTHENTICODE_SIGNATURE'],
+    )
+
 # Make sure python.exe can be located.
 win_env.AppendENVPath('PATH', os.environ['OMAHA_PYTHON_DIR'])