[tbb-commits] [tor-browser] 65/74: Bug 13379: Sign our MAR files.
gitolite role
git at cupani.torproject.org
Thu Oct 27 21:22:10 UTC 2022
This is an automated email from the git hooks/post-receive script.
richard pushed a commit to branch tor-browser-102.4.0esr-12.0-2
in repository tor-browser.
commit 1313ad23613e90a0c2c9425488dea1277a925073
Author: Kathy Brade <brade at pearlcrescent.com>
AuthorDate: 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.
Bug 19121: reinstate the update.xml hash check
Revert most changes from Mozilla Bug 1373267 "Remove hashFunction and
hashValue attributes from nsIUpdatePatch and code related to these
attributes." Changes to the tests were not reverted; the tests have
been changed significantly and we do not run automated updater tests
for Tor Browser at this time.
Also partial revert of commit f1241db6986e4b54473a1ed870f7584c75d51122.
Revert the nsUpdateService.js changes from Mozilla Bug 862173 "don't
verify mar file hash when using mar signing to verify the mar file
(lessens main thread I/O)."
Changes to the tests were not reverted; the tests have been changed
significantly and we do not run automated updater tests for
Tor Browser at this time.
We kept the addition to the AppConstants API in case other JS code
references it in the future.
---
browser/config/mozconfigs/tor-browser | 1 +
modules/libmar/tool/mar.c | 6 +-
modules/libmar/tool/moz.build | 12 +++-
modules/libmar/verify/moz.build | 14 ++--
toolkit/modules/AppConstants.jsm | 7 ++
toolkit/mozapps/update/UpdateService.jsm | 75 +++++++++++++++++++++-
toolkit/mozapps/update/UpdateTelemetry.jsm | 1 +
toolkit/mozapps/update/nsIUpdateService.idl | 11 ++++
.../mozapps/update/updater/updater-common.build | 24 ++++++-
toolkit/mozapps/update/updater/updater.cpp | 25 +++++---
toolkit/xre/moz.build | 3 +
toolkit/xre/nsUpdateDriver.cpp | 50 +++++++++++++++
12 files changed, 203 insertions(+), 26 deletions(-)
diff --git a/browser/config/mozconfigs/tor-browser b/browser/config/mozconfigs/tor-browser
index 8969a88aeaf9..0c10a3334cc2 100644
--- a/browser/config/mozconfigs/tor-browser
+++ b/browser/config/mozconfigs/tor-browser
@@ -5,5 +5,6 @@ mk_add_options MOZ_APP_DISPLAYNAME="Tor Browser"
ac_add_options --with-relative-profile=TorBrowser/Data/Browser
ac_add_options --enable-tor-browser-update
+ac_add_options --enable-verify-mar
ac_add_options --with-distribution-id=org.torproject
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 0f25dcc10aa4..d52f4415104d 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 CoreFoundation",
"-framework Security",
diff --git a/modules/libmar/verify/moz.build b/modules/libmar/verify/moz.build
index 17bf7b8387bb..202fbef4e06b 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/modules/AppConstants.jsm b/toolkit/modules/AppConstants.jsm
index 1738ad24512c..64695e4c996d 100644
--- a/toolkit/modules/AppConstants.jsm
+++ b/toolkit/modules/AppConstants.jsm
@@ -212,6 +212,13 @@ this.AppConstants = Object.freeze({
false,
#endif
+ MOZ_VERIFY_MAR_SIGNATURE:
+#ifdef MOZ_VERIFY_MAR_SIGNATURE
+ true,
+#else
+ false,
+#endif
+
MOZ_MAINTENANCE_SERVICE:
#ifdef MOZ_MAINTENANCE_SERVICE
true,
diff --git a/toolkit/mozapps/update/UpdateService.jsm b/toolkit/mozapps/update/UpdateService.jsm
index a10daf18d3de..501709856962 100644
--- a/toolkit/mozapps/update/UpdateService.jsm
+++ b/toolkit/mozapps/update/UpdateService.jsm
@@ -996,6 +996,21 @@ function LOG(string) {
}
}
+/**
+ * Convert a string containing binary values to hex.
+ */
+function binaryToHex(input) {
+ var result = "";
+ for (var i = 0; i < input.length; ++i) {
+ var hex = input.charCodeAt(i).toString(16);
+ if (hex.length == 1) {
+ hex = "0" + hex;
+ }
+ result += hex;
+ }
+ return result;
+}
+
/**
* Gets the specified directory at the specified hierarchy under the
* update root directory and creates it if it doesn't exist.
@@ -1941,6 +1956,8 @@ function UpdatePatch(patch) {
}
break;
case "finalURL":
+ case "hashFunction":
+ case "hashValue":
case "state":
case "type":
case "URL":
@@ -1960,6 +1977,8 @@ UpdatePatch.prototype = {
// over writing nsIUpdatePatch attributes.
_attrNames: [
"errorCode",
+ "hashFunction",
+ "hashValue",
"finalURL",
"selected",
"size",
@@ -1973,6 +1992,8 @@ UpdatePatch.prototype = {
*/
serialize: function UpdatePatch_serialize(updates) {
var patch = updates.createElementNS(URI_UPDATE_NS, "patch");
+ patch.setAttribute("hashFunction", this.hashFunction);
+ patch.setAttribute("hashValue", this.hashValue);
patch.setAttribute("size", this.size);
patch.setAttribute("type", this.type);
patch.setAttribute("URL", this.URL);
@@ -5157,7 +5178,53 @@ Downloader.prototype = {
}
LOG("Downloader:_verifyDownload downloaded size == expected size.");
- return true;
+ let fileStream = Cc[
+ "@mozilla.org/network/file-input-stream;1"
+ ].createInstance(Ci.nsIFileInputStream);
+ fileStream.init(
+ destination,
+ FileUtils.MODE_RDONLY,
+ FileUtils.PERMS_FILE,
+ 0
+ );
+
+ let digest;
+ try {
+ let hash = Cc["@mozilla.org/security/hash;1"].createInstance(
+ Ci.nsICryptoHash
+ );
+ var hashFunction =
+ Ci.nsICryptoHash[this._patch.hashFunction.toUpperCase()];
+ if (hashFunction == undefined) {
+ throw Components.Exception("", Cr.NS_ERROR_UNEXPECTED);
+ }
+ hash.init(hashFunction);
+ hash.updateFromStream(fileStream, -1);
+ // NOTE: For now, we assume that the format of _patch.hashValue is hex
+ // encoded binary (such as what is typically output by programs like
+ // sha1sum). In the future, this may change to base64 depending on how
+ // we choose to compute these hashes.
+ digest = binaryToHex(hash.finish(false));
+ } catch (e) {
+ LOG(
+ "Downloader:_verifyDownload - failed to compute hash of the downloaded update archive"
+ );
+ digest = "";
+ }
+
+ fileStream.close();
+
+ if (digest == this._patch.hashValue.toLowerCase()) {
+ LOG("Downloader:_verifyDownload hashes match.");
+ return true;
+ }
+
+ LOG("Downloader:_verifyDownload hashes do not match. ");
+ AUSTLMY.pingDownloadCode(
+ this.isCompleteUpdate,
+ AUSTLMY.DWNLD_ERR_VERIFY_NO_HASH_MATCH
+ );
+ return false;
},
/**
@@ -5738,6 +5805,9 @@ Downloader.prototype = {
" is higher than patch size: " +
this._patch.size
);
+ // It's important that we use a different code than
+ // NS_ERROR_CORRUPTED_CONTENT so that tests can verify the difference
+ // between a hash error and a wrong download error.
AUSTLMY.pingDownloadCode(
this.isCompleteUpdate,
AUSTLMY.DWNLD_ERR_PATCH_SIZE_LARGER
@@ -5756,6 +5826,9 @@ Downloader.prototype = {
" is not equal to expected patch size: " +
this._patch.size
);
+ // It's important that we use a different code than
+ // NS_ERROR_CORRUPTED_CONTENT so that tests can verify the difference
+ // between a hash error and a wrong download error.
AUSTLMY.pingDownloadCode(
this.isCompleteUpdate,
AUSTLMY.DWNLD_ERR_PATCH_SIZE_NOT_EQUAL
diff --git a/toolkit/mozapps/update/UpdateTelemetry.jsm b/toolkit/mozapps/update/UpdateTelemetry.jsm
index e75fd504866a..2612ed65529d 100644
--- a/toolkit/mozapps/update/UpdateTelemetry.jsm
+++ b/toolkit/mozapps/update/UpdateTelemetry.jsm
@@ -195,6 +195,7 @@ var AUSTLMY = {
DWNLD_ERR_VERIFY_NO_REQUEST: 13,
DWNLD_ERR_VERIFY_PATCH_SIZE_NOT_EQUAL: 14,
DWNLD_ERR_WRITE_FAILURE: 15,
+ DWNLD_ERR_VERIFY_NO_HASH_MATCH: 16,
// Temporary failure code to see if there are failures without an update phase
DWNLD_UNKNOWN_PHASE_ERR_WRITE_FAILURE: 40,
diff --git a/toolkit/mozapps/update/nsIUpdateService.idl b/toolkit/mozapps/update/nsIUpdateService.idl
index 668564822d24..ce1b40054de8 100644
--- a/toolkit/mozapps/update/nsIUpdateService.idl
+++ b/toolkit/mozapps/update/nsIUpdateService.idl
@@ -39,6 +39,17 @@ interface nsIUpdatePatch : nsISupports
*/
attribute AString finalURL;
+ /**
+ * The hash function to use when determining this file's integrity
+ */
+ attribute AString hashFunction;
+
+ /**
+ * The value of the hash function named above that should be computed if
+ * this file is not corrupt.
+ */
+ attribute AString hashValue;
+
/**
* The size of this file, in bytes.
*/
diff --git a/toolkit/mozapps/update/updater/updater-common.build b/toolkit/mozapps/update/updater/updater-common.build
index 7c58d374bbc2..4455b5093d73 100644
--- a/toolkit/mozapps/update/updater/updater-common.build
+++ b/toolkit/mozapps/update/updater/updater-common.build
@@ -4,6 +4,10 @@
# 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,14 +40,18 @@ if CONFIG["OS_ARCH"] == "WINNT":
"ws2_32",
"shell32",
"shlwapi",
- "crypt32",
- "advapi32",
"gdi32",
"user32",
"userenv",
"uuid",
]
+ if not link_with_nss:
+ OS_LIBS += [
+ "crypt32",
+ "advapi32",
+ ]
+
USE_LIBS += [
"bspatch",
"mar",
@@ -51,6 +59,13 @@ USE_LIBS += [
"xz-embedded",
]
+if link_with_nss:
+ USE_LIBS += [
+ "nspr",
+ "nss",
+ "signmar",
+ ]
+
if CONFIG["MOZ_WIDGET_TOOLKIT"] == "gtk":
have_progressui = 1
srcs += [
@@ -65,9 +80,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 2dc08e19957c..a0ddffaf0af9 100644
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -108,9 +108,11 @@ struct UpdateServerThreadArgs {
# define stat64 stat
#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"
@@ -2856,8 +2858,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);
}
}
}
@@ -3117,17 +3124,17 @@ int NS_main(int argc, NS_tchar** argv) {
if (!isDMGInstall) {
// Skip update-related code path for DMG installs.
-#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), (int)error);
_exit(1);
}
+# endif
#endif
// To process an update the updater command line must at a minimum have the
diff --git a/toolkit/xre/moz.build b/toolkit/xre/moz.build
index 6475c0296aac..a551edb8461f 100644
--- a/toolkit/xre/moz.build
+++ b/toolkit/xre/moz.build
@@ -232,6 +232,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 6808b5a5e9cf..4936114f579d 100644
--- a/toolkit/xre/nsUpdateDriver.cpp
+++ b/toolkit/xre/nsUpdateDriver.cpp
@@ -367,6 +367,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.
*
@@ -650,6 +686,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);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the tbb-commits
mailing list