From d7e38bb7e4f934f81eb52cd8ea5348680d391504 Mon Sep 17 00:00:00 2001
From: Robin Steuber <bytesized@mozilla.com>
Date: Fri, 21 Jun 2024 16:46:25 +0000
Subject: [PATCH] Bug 1902690 - More fully disable authenticode checks when
 using DISABLE_UPDATER_AUTHENTICODE_CHECK
 r=nalexander,application-update-reviewers

This should have no effect on any production code since no sane production configuration would turn this on. It is only for testing.

Differential Revision: https://phabricator.services.mozilla.com/D214212
---
 .../maintenanceservice/workmonitor.cpp        |  4 ---
 toolkit/mozapps/update/common/moz.build       |  3 ++
 .../update/common/registrycertificates.cpp    | 34 +++++++++++++++----
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/toolkit/components/maintenanceservice/workmonitor.cpp b/toolkit/components/maintenanceservice/workmonitor.cpp
index d417a73fede3..145f1363f8b1 100644
--- a/toolkit/components/maintenanceservice/workmonitor.cpp
+++ b/toolkit/components/maintenanceservice/workmonitor.cpp
@@ -394,11 +394,7 @@ static bool UpdaterIsValid(LPWSTR updater, LPWSTR installDir,
     return false;
   }
 
-#ifndef DISABLE_UPDATER_AUTHENTICODE_CHECK
   return DoesBinaryMatchAllowedCertificates(installDir, updater);
-#else
-  return true;
-#endif
 }
 
 /**
diff --git a/toolkit/mozapps/update/common/moz.build b/toolkit/mozapps/update/common/moz.build
index 2c79661c1f71..df32b227a4c7 100644
--- a/toolkit/mozapps/update/common/moz.build
+++ b/toolkit/mozapps/update/common/moz.build
@@ -32,6 +32,9 @@ Library("updatecommon")
 DEFINES["NS_NO_XPCOM"] = True
 USE_STATIC_LIBS = True
 
+if CONFIG["DISABLE_UPDATER_AUTHENTICODE_CHECK"]:
+    DEFINES["DISABLE_UPDATER_AUTHENTICODE_CHECK"] = True
+
 if CONFIG["OS_ARCH"] == "WINNT":
     # This forces the creation of updatecommon.lib, which the update agent needs
     # in order to link to updatecommon library functions.
diff --git a/toolkit/mozapps/update/common/registrycertificates.cpp b/toolkit/mozapps/update/common/registrycertificates.cpp
index 786218130a98..5b6cad6ac6cb 100644
--- a/toolkit/mozapps/update/common/registrycertificates.cpp
+++ b/toolkit/mozapps/update/common/registrycertificates.cpp
@@ -15,18 +15,38 @@
 /**
  * Verifies if the file path matches any certificate stored in the registry.
  *
- * @param  filePath The file path of the application to check if allowed.
- * @param  allowFallbackKeySkip when this is TRUE the fallback registry key will
- *   be used to skip the certificate check.  This is the default since the
- *   fallback registry key is located under HKEY_LOCAL_MACHINE which can't be
- *   written to by a low integrity process.
- *   Note: the maintenance service binary can be used to perform this check for
- *   testing or troubleshooting.
+ * @param  filePath
+ *         The file path of the application to check if allowed.
+ * @param  allowFallbackKeySkip
+ *         When this is TRUE the fallback registry key can be used to skip the
+ *         certificate check.  This is the default since the fallback registry
+ *         key is located under HKEY_LOCAL_MACHINE which can't be written to by
+ *         a low integrity process.
+ *         Note: The maintenance service binary can be used to perform this
+ *               check for testing or troubleshooting.
+ *         Note: When this is `TRUE` and we are building with
+ *               `DISABLE_UPDATER_AUTHENTICODE_CHECK`, this function will
+ *               unconditionally return `TRUE` since that flag is meant to
+ *               disable specifically this. We don't fall through in the `FALSE`
+ *               case since currently the only time when we don't allow the
+ *               fallback key is when we are running this for debugging purposes
+ *               and, in that case, it's more helpful if we return something
+ *               meaningful here.
+ *
  * @return TRUE if the binary matches any of the allowed certificates.
  */
 BOOL DoesBinaryMatchAllowedCertificates(LPCWSTR basePathForUpdate,
                                         LPCWSTR filePath,
                                         BOOL allowFallbackKeySkip) {
+#ifdef DISABLE_UPDATER_AUTHENTICODE_CHECK
+  if (allowFallbackKeySkip) {
+    LOG_WARN(("Skipping authenticode check"));
+    return TRUE;
+  } else {
+    LOG(("Performing a diagnostic authenticode check"));
+  }
+#endif
+
   WCHAR maintenanceServiceKey[MAX_PATH + 1];
   if (!CalculateRegistryPathFromFilePath(basePathForUpdate,
                                          maintenanceServiceKey)) {