Closed Bug 1158866 Opened 10 years ago Closed 10 years ago

Add support for MAR verification on Linux

Categories

(Toolkit :: Application Update, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: bbondy, Assigned: bbondy)

References

Details

Attachments

(1 file)

This bug is to make it possible for Linux to do MAR verification via NSS.  Most of this work was done in the context of bug 973933 but some problems remained, mainly in updater's use of shared libs because of the dynamic link to NSS.

This bug only aims to change Linux.
However if we ever want to enable NSS verification on OSX and Windows, it can be done with the following:

// OSX
#if defined(XP_MACOSX)
#define PATH_SEPARATOR ":"
#define LD_LIBRARY_PATH_ENVVAR_NAME "DYLD_LIBRARY_PATH"

// Windows
#if defined(XP_WIN)
#define PATH_SEPARATOR ";"
#define LD_LIBRARY_PATH_ENVVAR_NAME "PATH"

I don't see any advantage to doing it for other platforms at this time, and think keeping dependencies to a minimum of what's needed in updater as better for now by still using platform APIs.

The above is documented here just in case we ever decide to do it on OSX and Windows though.

There will be a separate bug to actually define MOZ_VERIFY_MAR_SIGNATURE where it is needed since that can hold things up and I'd like to get this landed.
Attached patch Patch rev1.Splinter Review
Linux related XPCShell tests are passing.

I previously was testing with firefox-40.0a1.en-US.linux-x86_64.tar.bz2 on Ubuntu and getting failed updates.
I just verified and it’s now working as well. 


The `OS_LIBS += CONFIG['NSPR_LIBS']` change was needed or else updater gets a linking error when MOZ_VERIFY_MAR_SIGNATURE is defined on Linux.

Posting a separate bug shortly which will be linked to the same multi platform MAR verification tracking bug to actually define MOZ_VERIFY_MAR_SIGNATURE at the best spot possible.
Attachment #8598060 - Flags: review?(robert.strong.bugs)
Comment on attachment 8598060 [details] [diff] [review]
Patch rev1.

>diff --git a/toolkit/xre/nsUpdateDriver.cpp b/toolkit/xre/nsUpdateDriver.cpp
>--- a/toolkit/xre/nsUpdateDriver.cpp
>+++ b/toolkit/xre/nsUpdateDriver.cpp
>@@ -395,16 +395,46 @@ CopyUpdaterIntoUpdateDir(nsIFile *greDir
>   if (NS_FAILED(tmp) || NS_FAILED(rv))
>     return false;
> #endif
>   rv = updater->AppendNative(NS_LITERAL_CSTRING(kUpdaterBin));
>   return NS_SUCCEEDED(rv); 
> }
> 
> /**
>+ * Appends the specified path to the library path.
>+ * This is used so that updater can find libmozsqlite3.so and other shared libs.
>+ *
>+ * @param newPathToCheck A new library path to prepend to LD_LIBRARY_PATH
>+ */
>+#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
>+#include "prprf.h"
>+#define PATH_SEPARATOR ":"
>+#define LD_LIBRARY_PATH_ENVVAR_NAME "LD_LIBRARY_PATH"
>+static void
>+AppendToLibPath(const char *newPathToCheck)
nitL s/newPathToCheck/pathToAppend/

>+{
>+  char *s = nullptr;
>+  char *pathValue = getenv(LD_LIBRARY_PATH_ENVVAR_NAME);
>+  if (nullptr == pathValue || '\0' == *pathValue) {
>+    s = PR_smprintf("%s=%s", LD_LIBRARY_PATH_ENVVAR_NAME, newPathToCheck);
>+  } else {
>+    s = PR_smprintf("%s=%s" PATH_SEPARATOR "%s",
>+                    LD_LIBRARY_PATH_ENVVAR_NAME, newPathToCheck, pathValue);
>+  }
>+
>+  // The memory used by PR_SetEnv is not copied to the environment on all
>+  // platform, it can be used by reference directly. So we purposely do not
>+  // call PR_smprintf_free on s.  Subsequent calls to PR_SetEnv will free
>+  // the old memory first.
>+  PR_SetEnv(s);
>+}
>+#endif
>+
>+/**
>  * Switch an existing application directory to an updated version that has been
>  * staged.
>  *
>  * @param greDir the GRE dir
>  * @param updateDir the update dir where the mar file is located
>  * @param appDir the app dir
>  * @param appArgc the number of args to the application
>  * @param appArgv the args to the application, used for restarting if needed
>@@ -580,16 +610,19 @@ SwitchToUpdatedApp(nsIFile *greDir, nsIF
>   } else {
>     argc = 5;
>     argv[5] = nullptr;
>   }
> 
>   if (gSafeMode) {
>     PR_SetEnv("MOZ_SAFE_MODE_RESTART=1");
>   }
>+#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
>+  AppendToLibPath(installDirPath.get());
>+#endif
> 
>   LOG(("spawning updater process for replacing [%s]\n", updaterPath.get()));
> 
> #if defined(USE_EXECV)
> # if defined(MOZ_WIDGET_GONK)
>   // In Gonk, we preload libmozglue, which the updater process doesn't need.
>   // Since the updater will move and delete libmozglue.so, this can actually
>   // stop the /system mount from correctly being remounted as read-only.
>@@ -845,16 +878,19 @@ ApplyUpdate(nsIFile *greDir, nsIFile *up
>   } else {
>     argc = 5;
>     argv[5] = nullptr;
>   }
> 
>   if (gSafeMode) {
>     PR_SetEnv("MOZ_SAFE_MODE_RESTART=1");
>   }
>+#if defined(MOZ_VERIFY_MAR_SIGNATURE) && !defined(XP_WIN) && !defined(XP_MACOSX)
>+  AppendToLibPath(installDirPath.get());
>+#endif
> 
>   if (isOSUpdate) {
>     PR_SetEnv("MOZ_OS_UPDATE=1");
>   }
> #if defined(MOZ_WIDGET_GONK)
>   // We want the updater to be CPU friendly and not subject to being killed by
>   // the low memory killer, so we pass in some preferences to allow it to
>   // adjust its priority.
Attachment #8598060 - Flags: review?(robert.strong.bugs) → review+
Target Milestone: --- → mozilla40
Depends on: 1159090
https://hg.mozilla.org/mozilla-central/rev/27eafcbadfda
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: