[tbb-commits] [tor-browser/tor-browser-84.0.2-10.0-1] Bug 13379: Sign our MAR files.

sysrqb at torproject.org sysrqb at torproject.org
Fri Jan 8 16:14:03 UTC 2021


commit 2e99453c8b442a504a244dac03a67a5bed6a090e
Author: Kathy Brade <brade at pearlcrescent.com>
Date:   Wed Dec 17 16:37:11 2014 -0500

    Bug 13379: Sign our MAR files.
    
    Configure with --enable-verify-mar (when updating, require a valid
      signature on the MAR file before it is applied).
    Use the Tor Browser version instead of the Firefox version inside the
      MAR file info block (necessary to prevent downgrade attacks).
    Use NSS on all platforms for checking MAR signatures (instead of using
      OS-native APIs, which Mozilla does on Mac OS and Windows). So that the
      NSS and NSPR libraries the updater depends on can be found at runtime,
      we add the firefox directory to the shared library search path on macOS.
      On Linux, rpath is used by Mozilla to solve that problem, but that
      approach won't work on macOS because the updater executable is copied
      during the update process to a location that is under TorBrowser-Data,
      and the location of TorBrowser-Data varies.
    
    Also includes the fix for bug 18900.
---
 .mozconfig                                         |  1 +
 .mozconfig-asan                                    |  1 +
 .mozconfig-mac                                     |  1 +
 .mozconfig-mingw                                   |  1 +
 modules/libmar/tool/mar.c                          |  6 +--
 modules/libmar/tool/moz.build                      | 12 ++++--
 modules/libmar/verify/moz.build                    | 14 +++---
 .../mozapps/update/updater/updater-common.build    | 26 +++++++++--
 toolkit/mozapps/update/updater/updater.cpp         | 25 +++++++----
 toolkit/xre/moz.build                              |  3 ++
 toolkit/xre/nsUpdateDriver.cpp                     | 50 ++++++++++++++++++++++
 11 files changed, 115 insertions(+), 25 deletions(-)

diff --git a/.mozconfig b/.mozconfig
index 24efaea57b0b..d71c858844e3 100755
--- a/.mozconfig
+++ b/.mozconfig
@@ -36,3 +36,4 @@ ac_add_options MOZ_TELEMETRY_REPORTING=
 ac_add_options --disable-tor-launcher
 ac_add_options --with-tor-browser-version=dev-build
 ac_add_options --disable-tor-browser-update
+ac_add_options --enable-verify-mar
diff --git a/.mozconfig-asan b/.mozconfig-asan
index 13232e054d45..ca05fb12eedb 100644
--- a/.mozconfig-asan
+++ b/.mozconfig-asan
@@ -28,6 +28,7 @@ ac_add_options --enable-official-branding
 ac_add_options --enable-default-toolkit=cairo-gtk3
 
 ac_add_options --enable-tor-browser-update
+ac_add_options --enable-verify-mar
 
 ac_add_options --disable-strip
 ac_add_options --disable-install-strip
diff --git a/.mozconfig-mac b/.mozconfig-mac
index 1f89cab30bbc..9be7751f8241 100644
--- a/.mozconfig-mac
+++ b/.mozconfig-mac
@@ -42,6 +42,7 @@ ac_add_options --disable-debug
 
 ac_add_options --enable-tor-browser-data-outside-app-dir
 ac_add_options --enable-tor-browser-update
+ac_add_options --enable-verify-mar
 
 ac_add_options --disable-crashreporter
 ac_add_options --disable-webrtc
diff --git a/.mozconfig-mingw b/.mozconfig-mingw
index 4fb050308060..29c58d8fdab2 100644
--- a/.mozconfig-mingw
+++ b/.mozconfig-mingw
@@ -14,6 +14,7 @@ ac_add_options --enable-strip
 ac_add_options --enable-official-branding
 
 ac_add_options --enable-tor-browser-update
+ac_add_options --enable-verify-mar
 ac_add_options --disable-bits-download
 
 # Let's make sure no preference is enabling either Adobe's or Google's CDM.
diff --git a/modules/libmar/tool/mar.c b/modules/libmar/tool/mar.c
index 0bf2cb4bd1d4..ea2b79924914 100644
--- a/modules/libmar/tool/mar.c
+++ b/modules/libmar/tool/mar.c
@@ -65,7 +65,7 @@ static void print_usage() {
       "signed_input_archive.mar base_64_encoded_signature_file "
       "changed_signed_output.mar\n");
   printf("(i) is the index of the certificate to extract\n");
-#  if defined(XP_MACOSX) || (defined(XP_WIN) && !defined(MAR_NSS))
+#  if (defined(XP_MACOSX) || defined(XP_WIN)) && !defined(MAR_NSS)
   printf("Verify a MAR file:\n");
   printf("  mar [-C workingDir] -D DERFilePath -v signed_archive.mar\n");
   printf(
@@ -149,7 +149,7 @@ int main(int argc, char** argv) {
   memset((void*)certBuffers, 0, sizeof(certBuffers));
 #endif
 #if !defined(NO_SIGN_VERIFY) && \
-    ((!defined(MAR_NSS) && defined(XP_WIN)) || defined(XP_MACOSX))
+    (!defined(MAR_NSS) && (defined(XP_WIN) || defined(XP_MACOSX)))
   memset(DERFilePaths, 0, sizeof(DERFilePaths));
   memset(fileSizes, 0, sizeof(fileSizes));
 #endif
@@ -181,7 +181,7 @@ int main(int argc, char** argv) {
       argc -= 2;
     }
 #if !defined(NO_SIGN_VERIFY)
-#  if (!defined(MAR_NSS) && defined(XP_WIN)) || defined(XP_MACOSX)
+#  if (!defined(MAR_NSS) && (defined(XP_WIN) || defined(XP_MACOSX)))
     /* -D DERFilePath, also matches -D[index] DERFilePath
        We allow an index for verifying to be symmetric
        with the import and export command line arguments. */
diff --git a/modules/libmar/tool/moz.build b/modules/libmar/tool/moz.build
index a6d26c66a668..d6fa1677ddf1 100644
--- a/modules/libmar/tool/moz.build
+++ b/modules/libmar/tool/moz.build
@@ -43,15 +43,21 @@ if CONFIG["MOZ_BUILD_APP"] != "tools/update-packaging":
         "verifymar",
     ]
 
+    if CONFIG["TOR_BROWSER_UPDATE"]:
+        DEFINES["MAR_NSS"] = True
+
     if CONFIG["OS_ARCH"] == "WINNT":
         USE_STATIC_LIBS = True
 
         OS_LIBS += [
             "ws2_32",
-            "crypt32",
-            "advapi32",
         ]
-    elif CONFIG["OS_ARCH"] == "Darwin":
+        if not CONFIG["TOR_BROWSER_UPDATE"]:
+            OS_LIBS += [
+                "crypt32",
+                "advapi32",
+            ]
+    elif CONFIG["OS_ARCH"] == "Darwin" and not CONFIG["TOR_BROWSER_UPDATE"]:
         OS_LIBS += [
             "-framework Security",
         ]
diff --git a/modules/libmar/verify/moz.build b/modules/libmar/verify/moz.build
index b07475655f0d..03718eee50b4 100644
--- a/modules/libmar/verify/moz.build
+++ b/modules/libmar/verify/moz.build
@@ -16,15 +16,12 @@ FORCE_STATIC_LIB = True
 if CONFIG["OS_ARCH"] == "WINNT":
     USE_STATIC_LIBS = True
 elif CONFIG["OS_ARCH"] == "Darwin":
-    UNIFIED_SOURCES += [
-        "MacVerifyCrypto.cpp",
-    ]
-    OS_LIBS += [
-        "-framework Security",
+    USE_LIBS += [
+        "nspr",
+        "nss",
+        "signmar",
     ]
 else:
-    DEFINES["MAR_NSS"] = True
-    LOCAL_INCLUDES += ["../sign"]
     USE_LIBS += [
         "nspr",
         "nss",
@@ -38,6 +35,9 @@ else:
         "-Wl,-rpath=\\$$ORIGIN",
     ]
 
+DEFINES["MAR_NSS"] = True
+LOCAL_INCLUDES += ["../sign"]
+
 LOCAL_INCLUDES += [
     "../src",
 ]
diff --git a/toolkit/mozapps/update/updater/updater-common.build b/toolkit/mozapps/update/updater/updater-common.build
index 2f2a210f255b..74b24151757f 100644
--- a/toolkit/mozapps/update/updater/updater-common.build
+++ b/toolkit/mozapps/update/updater/updater-common.build
@@ -4,6 +4,12 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+DEFINES["MAR_NSS"] = True
+
+link_with_nss = DEFINES["MAR_NSS"] or (
+    CONFIG["OS_ARCH"] == "Linux" and CONFIG["MOZ_VERIFY_MAR_SIGNATURE"]
+)
+
 srcs = [
     "archivereader.cpp",
     "updater.cpp",
@@ -36,10 +42,14 @@ if CONFIG["OS_ARCH"] == "WINNT":
         "ws2_32",
         "shell32",
         "shlwapi",
-        "crypt32",
-        "advapi32",
     ]
 
+    if not link_with_nss:
+        OS_LIBS += [
+            "crypt32",
+            "advapi32",
+        ]
+
 USE_LIBS += [
     "bspatch",
     "mar",
@@ -47,6 +57,13 @@ USE_LIBS += [
     "xz-embedded",
 ]
 
+if link_with_nss:
+    USE_LIBS += [
+        "nspr",
+        "nss",
+        "signmar",
+    ]
+
 if CONFIG["MOZ_WIDGET_TOOLKIT"] == "gtk":
     have_progressui = 1
     srcs += [
@@ -61,9 +78,12 @@ if CONFIG["MOZ_WIDGET_TOOLKIT"] == "cocoa":
     ]
     OS_LIBS += [
         "-framework Cocoa",
-        "-framework Security",
         "-framework SystemConfiguration",
     ]
+    if not link_with_nss:
+        OS_LIBS += [
+            "-framework Security",
+        ]
     UNIFIED_SOURCES += [
         "/toolkit/xre/updaterfileutils_osx.mm",
     ]
diff --git a/toolkit/mozapps/update/updater/updater.cpp b/toolkit/mozapps/update/updater/updater.cpp
index 226354d5e753..d6f1f6c568bc 100644
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -106,9 +106,11 @@ struct UpdateServerThreadArgs {
 #  define USE_EXECV
 #endif
 
-#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
-#  include "nss.h"
-#  include "prerror.h"
+#if defined(MOZ_VERIFY_MAR_SIGNATURE)
+#  if defined(MAR_NSS) || (!defined(XP_WIN) && !defined(XP_MACOSX))
+#    include "nss.h"
+#    include "prerror.h"
+#  endif
 #endif
 
 #include "crctable.h"
@@ -2721,8 +2723,13 @@ static void UpdateThreadFunc(void* param) {
         if (ReadMARChannelIDs(updateSettingsPath, &MARStrings) != OK) {
           rv = UPDATE_SETTINGS_FILE_CHANNEL;
         } else {
+#  ifdef TOR_BROWSER_UPDATE
+          const char* appVersion = TOR_BROWSER_VERSION_QUOTED;
+#  else
+          const char* appVersion = MOZ_APP_VERSION;
+#  endif
           rv = gArchiveReader.VerifyProductInformation(
-              MARStrings.MARChannelID.get(), MOZ_APP_VERSION);
+              MARStrings.MARChannelID.get(), appVersion);
         }
       }
     }
@@ -2923,11 +2930,10 @@ int NS_main(int argc, NS_tchar** argv) {
   }
 #endif
 
-#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
-  // On Windows and Mac we rely on native APIs to do verifications so we don't
-  // need to initialize NSS at all there.
-  // Otherwise, minimize the amount of NSS we depend on by avoiding all the NSS
-  // databases.
+#if defined(MOZ_VERIFY_MAR_SIGNATURE)
+#  if defined(MAR_NSS) || (!defined(XP_WIN) && !defined(XP_MACOSX))
+  // If using NSS for signature verification, initialize NSS but minimize
+  // the portion we depend on by avoiding all of the NSS databases.
   if (NSS_NoDB_Init(nullptr) != SECSuccess) {
     PRErrorCode error = PR_GetError();
     fprintf(stderr, "Could not initialize NSS: %s (%d)", PR_ErrorToName(error),
@@ -2935,6 +2941,7 @@ int NS_main(int argc, NS_tchar** argv) {
     _exit(1);
   }
 #endif
+#endif
 
 #ifdef XP_MACOSX
   if (!isElevated) {
diff --git a/toolkit/xre/moz.build b/toolkit/xre/moz.build
index 8f80bf7a9d95..e4a1f54e495c 100644
--- a/toolkit/xre/moz.build
+++ b/toolkit/xre/moz.build
@@ -225,6 +225,9 @@ for var in ("APP_VERSION", "APP_ID"):
 if CONFIG["MOZ_BUILD_APP"] == "browser":
     DEFINES["MOZ_BUILD_APP_IS_BROWSER"] = True
 
+if CONFIG["TOR_BROWSER_UPDATE"]:
+    DEFINES["MAR_NSS"] = True
+
 LOCAL_INCLUDES += [
     "../../other-licenses/nsis/Contrib/CityHash/cityhash",
     "../components/find",
diff --git a/toolkit/xre/nsUpdateDriver.cpp b/toolkit/xre/nsUpdateDriver.cpp
index 9e128b0b04c7..8df2d54195f7 100644
--- a/toolkit/xre/nsUpdateDriver.cpp
+++ b/toolkit/xre/nsUpdateDriver.cpp
@@ -360,6 +360,42 @@ static nsresult GetUpdateDirFromAppDir(nsIFile* aAppDir, nsIFile** aResult) {
 #  endif
 #endif
 
+#if defined(TOR_BROWSER_UPDATE) && defined(MOZ_VERIFY_MAR_SIGNATURE) && \
+    defined(MAR_NSS) && defined(XP_MACOSX)
+/**
+ * Ideally we would save and restore the original library path value after
+ * the updater finishes its work (and before firefox is re-launched).
+ * Doing so would avoid potential problems like the following bug:
+ *   https://bugzilla.mozilla.org/show_bug.cgi?id=1434033
+ */
+/**
+ * Appends the specified path to the library path.
+ * This is used so that the updater can find libnss3.dylib and other
+ * shared libs.
+ *
+ * @param pathToAppend A new library path to prepend to the dynamic linker's
+ * search path.
+ */
+#  include "prprf.h"
+#  define PATH_SEPARATOR ":"
+#  define LD_LIBRARY_PATH_ENVVAR_NAME "DYLD_LIBRARY_PATH"
+static void AppendToLibPath(const char* pathToAppend) {
+  char* pathValue = getenv(LD_LIBRARY_PATH_ENVVAR_NAME);
+  if (nullptr == pathValue || '\0' == *pathValue) {
+    // Leak the string because that is required by PR_SetEnv.
+    char* s =
+        Smprintf("%s=%s", LD_LIBRARY_PATH_ENVVAR_NAME, pathToAppend).release();
+    PR_SetEnv(s);
+  } else {
+    // Leak the string because that is required by PR_SetEnv.
+    char* s = Smprintf("%s=%s" PATH_SEPARATOR "%s", LD_LIBRARY_PATH_ENVVAR_NAME,
+                       pathToAppend, pathValue)
+                  .release();
+    PR_SetEnv(s);
+  }
+}
+#endif
+
 /**
  * Applies, switches, or stages an update.
  *
@@ -606,6 +642,20 @@ static void ApplyUpdate(nsIFile* greDir, nsIFile* updateDir, nsIFile* appDir,
     PR_SetEnv("MOZ_SAFE_MODE_RESTART=1");
   }
 
+#if defined(TOR_BROWSER_UPDATE) && defined(MOZ_VERIFY_MAR_SIGNATURE) && \
+    defined(MAR_NSS) && defined(XP_MACOSX)
+  // On macOS, append the app directory to the shared library search path
+  // so the system can locate the shared libraries that are needed by the
+  // updater, e.g., libnss3.dylib).
+  nsAutoCString appPath;
+  nsresult rv2 = appDir->GetNativePath(appPath);
+  if (NS_SUCCEEDED(rv2)) {
+    AppendToLibPath(appPath.get());
+  } else {
+    LOG(("ApplyUpdate -- appDir->GetNativePath() failed (0x%x)\n", rv2));
+  }
+#endif
+
   LOG(("spawning updater process [%s]\n", updaterPath.get()));
 #ifdef DEBUG
   dump_argv("ApplyUpdate updater", argv, argc);





More information about the tbb-commits mailing list