From 14d4144b58c78688f81ba6a8bf4eb888f975975b Mon Sep 17 00:00:00 2001
From: Pier Angelo Vendrame <pierov@torproject.org>
Date: Wed, 14 Jun 2023 16:36:04 +0000
Subject: [PATCH] Bug 1832523 - Allow using NSS to sign and verify MAR
signatures. r=application-update-reviewers,glandium,bytesized
Allow using NSS for checking MAR signatures also in platforms where
OS-native APIs are used by default, i.e., macOS and Windows.
Differential Revision: https://phabricator.services.mozilla.com/D177743
---
build/moz.configure/update-programs.configure | 23 ++++++++++++++
modules/libmar/tool/mar.c | 31 +++++++++----------
modules/libmar/tool/moz.build | 12 +++++--
modules/libmar/verify/moz.build | 23 ++++++++------
.../update/updater/updater-common.build | 28 +++++++++++++++--
toolkit/mozapps/update/updater/updater.cpp | 10 +++---
toolkit/xre/moz.build | 3 ++
7 files changed, 92 insertions(+), 38 deletions(-)
diff --git a/build/moz.configure/update-programs.configure b/build/moz.configure/update-programs.configure
index 07a07f41b5f18..bcdd457ad3397 100644
--- a/build/moz.configure/update-programs.configure
+++ b/build/moz.configure/update-programs.configure
@@ -54,6 +54,29 @@ set_config(
depends_if("--enable-unverified-updates")(lambda _: True),
)
+# Use NSS for MAR signatures even on platforms where system libraries are
+# supported (currently Windows and macOS).
+# ==============================================================
+
+can_toggle_nss_mar = target_is_windows | target_is_osx
+
+option(
+ "--enable-nss-mar",
+ when=can_toggle_nss_mar,
+ help="Enable using NSS to check MAR signatures instead of system crypto.",
+)
+
+
+@depends(
+ depends("--enable-nss-mar", when=can_toggle_nss_mar)(lambda x: x),
+ can_toggle_nss_mar,
+)
+def enable_nss_mar(enabled, can_toggle_nss_mar):
+ return enabled or not can_toggle_nss_mar
+
+
+set_config("MOZ_USE_NSS_FOR_MAR", True, when=enable_nss_mar)
+
# Maintenance service (Windows only)
# ==============================================================
diff --git a/modules/libmar/tool/mar.c b/modules/libmar/tool/mar.c
index 449c9f7efc202..a5cf7ba6eb186 100644
--- a/modules/libmar/tool/mar.c
+++ b/modules/libmar/tool/mar.c
@@ -18,7 +18,7 @@
# include <unistd.h>
#endif
-#if !defined(NO_SIGN_VERIFY) && (!defined(XP_WIN) || defined(MAR_NSS))
+#if !defined(NO_SIGN_VERIFY) && defined(MAR_NSS)
# include "cert.h"
# include "nss.h"
# include "pk11pub.h"
@@ -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(MAR_NSS)
printf("Verify a MAR file:\n");
printf(" mar [-C workingDir] -D DERFilePath -v signed_archive.mar\n");
printf(
@@ -135,23 +135,22 @@ int main(int argc, char** argv) {
int32_t sigIndex = -1;
uint32_t fileSizes[MAX_SIGNATURES];
const uint8_t* certBuffers[MAX_SIGNATURES];
-# if ((!defined(MAR_NSS) && defined(XP_WIN)) || defined(XP_MACOSX)) || \
- ((defined(XP_WIN) || defined(XP_MACOSX)) && !defined(MAR_NSS))
+# if !defined(MAR_NSS)
char* DERFilePaths[MAX_SIGNATURES];
-# endif
-# if (!defined(XP_WIN) && !defined(XP_MACOSX)) || defined(MAR_NSS)
+# else
CERTCertificate* certs[MAX_SIGNATURES];
# endif
#endif
memset((void*)certNames, 0, sizeof(certNames));
-#if defined(XP_WIN) && !defined(MAR_NSS) && !defined(NO_SIGN_VERIFY)
+#if !defined(NO_SIGN_VERIFY)
+ memset(fileSizes, 0, sizeof(fileSizes));
memset((void*)certBuffers, 0, sizeof(certBuffers));
-#endif
-#if !defined(NO_SIGN_VERIFY) && \
- ((!defined(MAR_NSS) && defined(XP_WIN)) || defined(XP_MACOSX))
+# if !defined(MAR_NSS)
memset(DERFilePaths, 0, sizeof(DERFilePaths));
- memset(fileSizes, 0, sizeof(fileSizes));
+# else
+ memset(certs, 0, sizeof(certs));
+# endif
#endif
if (argc > 1 && 0 == strcmp(argv[1], "--version")) {
@@ -181,7 +180,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)
/* -D DERFilePath, also matches -D[index] DERFilePath
We allow an index for verifying to be symmetric
with the import and export command line arguments. */
@@ -348,7 +347,7 @@ int main(int argc, char** argv) {
return -1;
}
-# if (!defined(XP_WIN) && !defined(XP_MACOSX)) || defined(MAR_NSS)
+# if defined(MAR_NSS)
if (!NSSConfigDir || certCount == 0) {
print_usage();
return -1;
@@ -362,7 +361,7 @@ int main(int argc, char** argv) {
rv = 0;
for (k = 0; k < certCount; ++k) {
-# if (defined(XP_WIN) || defined(XP_MACOSX)) && !defined(MAR_NSS)
+# if !defined(MAR_NSS)
rv = mar_read_entire_file(DERFilePaths[k], MAR_MAX_CERT_SIZE,
&certBuffers[k], &fileSizes[k]);
@@ -404,7 +403,7 @@ int main(int argc, char** argv) {
}
}
for (k = 0; k < certCount; ++k) {
-# if (defined(XP_WIN) || defined(XP_MACOSX)) && !defined(MAR_NSS)
+# if !defined(MAR_NSS)
free((void*)certBuffers[k]);
# else
/* certBuffers[k] is owned by certs[k] so don't free it */
@@ -423,7 +422,7 @@ int main(int argc, char** argv) {
" no signature to verify.\n");
}
}
-# if (!defined(XP_WIN) && !defined(XP_MACOSX)) || defined(MAR_NSS)
+# if defined(MAR_NSS)
(void)NSS_Shutdown();
# endif
return rv ? -1 : 0;
diff --git a/modules/libmar/tool/moz.build b/modules/libmar/tool/moz.build
index 0f25dcc10aa45..b2772544d50d1 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["MOZ_USE_NSS_FOR_MAR"]:
+ 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["MOZ_USE_NSS_FOR_MAR"]:
+ OS_LIBS += [
+ "crypt32",
+ "advapi32",
+ ]
+ elif CONFIG["OS_ARCH"] == "Darwin" and not CONFIG["MOZ_USE_NSS_FOR_MAR"]:
OS_LIBS += [
"-framework CoreFoundation",
"-framework Security",
diff --git a/modules/libmar/verify/moz.build b/modules/libmar/verify/moz.build
index b07475655f0dc..5ea079a666bbb 100644
--- a/modules/libmar/verify/moz.build
+++ b/modules/libmar/verify/moz.build
@@ -15,7 +15,7 @@ FORCE_STATIC_LIB = True
if CONFIG["OS_ARCH"] == "WINNT":
USE_STATIC_LIBS = True
-elif CONFIG["OS_ARCH"] == "Darwin":
+elif CONFIG["OS_ARCH"] == "Darwin" and not CONFIG["MOZ_USE_NSS_FOR_MAR"]:
UNIFIED_SOURCES += [
"MacVerifyCrypto.cpp",
]
@@ -23,25 +23,28 @@ elif CONFIG["OS_ARCH"] == "Darwin":
"-framework Security",
]
else:
- DEFINES["MAR_NSS"] = True
- LOCAL_INCLUDES += ["../sign"]
USE_LIBS += [
"nspr",
"nss",
"signmar",
]
- # Ideally, this would be '-Wl,-rpath=$ORIGIN', but the build system
- # doesn't do the right escaping yet. Even more ideally, this would
- # be LDFLAGS, but the build system doesn't propagate those like USE_LIBS
- # and OS_LIBS. Bug #1041943.
- OS_LIBS += [
- "-Wl,-rpath=\\$$ORIGIN",
- ]
+ if CONFIG["OS_ARCH"] != "Darwin":
+ # Ideally, this would be '-Wl,-rpath=$ORIGIN', but the build system
+ # doesn't do the right escaping yet. Even more ideally, this would
+ # be LDFLAGS, but the build system doesn't propagate those like
+ # USE_LIBS and OS_LIBS. Bug #1041943.
+ OS_LIBS += [
+ "-Wl,-rpath=\\$$ORIGIN",
+ ]
LOCAL_INCLUDES += [
"../src",
]
+if CONFIG["MOZ_USE_NSS_FOR_MAR"]:
+ LOCAL_INCLUDES += ["../sign"]
+ DEFINES["MAR_NSS"] = True
+
# C11 for static_assert
c11_flags = ["-std=gnu11"]
if CONFIG["CC_TYPE"] == "clang-cl":
diff --git a/toolkit/mozapps/update/updater/updater-common.build b/toolkit/mozapps/update/updater/updater-common.build
index 16c2d1500392a..fe0b4a85faa11 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/.
+link_with_nss = CONFIG["MOZ_USE_NSS_FOR_MAR"] or (
+ CONFIG["OS_ARCH"] == "Linux" and CONFIG["MOZ_VERIFY_MAR_SIGNATURE"]
+)
+if link_with_nss:
+ DEFINES["MAR_NSS"] = True
+
srcs = [
"archivereader.cpp",
"updater.cpp",
@@ -36,14 +42,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 +61,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 +82,14 @@ if CONFIG["MOZ_WIDGET_TOOLKIT"] == "cocoa":
]
OS_LIBS += [
"-framework Cocoa",
- "-framework Security",
"-framework SystemConfiguration",
]
+ if link_with_nss:
+ LDFLAGS += ["-Wl,-rpath,@executable_path/../../../"]
+ else:
+ 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 732416c7ce6f1..446f1578eeb20 100644
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -107,7 +107,7 @@ struct UpdateServerThreadArgs {
# define stat64 stat
#endif
-#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
+#if defined(MOZ_VERIFY_MAR_SIGNATURE) && defined(MAR_NSS)
# include "nss.h"
# include "prerror.h"
#endif
@@ -2892,11 +2892,9 @@ 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) && defined(MAR_NSS)
+ // 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)",
diff --git a/toolkit/xre/moz.build b/toolkit/xre/moz.build
index dc7695e7acc5f..21de5bce3422c 100644
--- a/toolkit/xre/moz.build
+++ b/toolkit/xre/moz.build
@@ -228,6 +228,9 @@ for var in ("APP_VERSION", "APP_ID"):
if CONFIG["MOZ_BUILD_APP"] == "browser":
DEFINES["MOZ_BUILD_APP_IS_BROWSER"] = True
+if CONFIG["MOZ_USE_NSS_FOR_MAR"]:
+ DEFINES["MAR_NSS"] = True
+
LOCAL_INCLUDES += [
"../../other-licenses/nsis/Contrib/CityHash/cityhash",
"../components/find",
--
GitLab