Closed
Bug 731382
Opened 12 years ago
Closed 12 years ago
Port security enhancements for silent updates to Aurora
Categories
(Toolkit :: Application Update, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla12
Tracking | Status | |
---|---|---|
firefox12 | --- | fixed |
People
(Reporter: bbondy, Assigned: bbondy)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file, 2 obsolete files)
347.47 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
This bug is to port the security enhancements that the security team said is needed before the service can land on Mozilla-Beta. All of these bugs landed on v13 but need to be on v12 Aurora before the next migration. The sooner this can land the better because more testing can be done on Aurora before it reaches beta. Also there is a test day this Friday for Aurora and I'd like to get 2 Nightly builds in before then. Almost everything stayed 100% the same as what's on mozilla-central (no rebasing). The only change is the MAR_CHANNEL_ID and ACCEPTED_MAR_CHANNEL_IDS values in browser/confvars.sh which needed to be adjusted to: firefox-mozilla-aurora. --- From oldest to newest the changesets included in this patch are: http://hg.mozilla.org/mozilla-central/rev/033ca7c5bce8 Bug 699700 - Add support for signing and verifying MAR files in libmar and the mar program. http://hg.mozilla.org/mozilla-central/rev/94f7927299e5 Bug 704285 - Include certificates inside updater.exe and use them to verify MARs. http://hg.mozilla.org/mozilla-central/rev/58402b43c9e1 Bug 704285 - Certificates for updater cert check. http://hg.mozilla.org/mozilla-central/rev/ef967515537b Bug 711139 - MOZ_VERIFY_MAR_SIGNATURE build option for verifying MAR signatures. http://hg.mozilla.org/mozilla-central/rev/57ab36828a9e Bug 708690 - libmar enhancements for product information blocks. http://hg.mozilla.org/mozilla-central/rev/4de2d63b21cf Bug 708690 - Updater enhancements for product information blocks. http://hg.mozilla.org/mozilla-central/rev/c7f5af7dad8d Bug 708690 - Build config for product information blocks. http://hg.mozilla.org/mozilla-central/rev/ad8fe9d36d16 Bug 720688 - Ability to strip MAR signatures in libmar. http://hg.mozilla.org/mozilla-central/rev/df3801e30225 Bug 708690 - XPCShell test enhancements for product information blocks. http://hg.mozilla.org/mozilla-central/rev/52bd72d77fda Bug 708690 - Updated MAR files for product information blocks. http://hg.mozilla.org/mozilla-central/rev/d74612c58245 Bug 721758 - Ability to configure updater to accept multiple MAR IDs. http://hg.mozilla.org/mozilla-central/rev/0bde78f1b76a Bug 728301 - Enable new security checks only for the service. http://hg.mozilla.org/mozilla-central/rev/bfb987083bcb Bug 725180 - Create test suite for libmar. http://hg.mozilla.org/mozilla-central/rev/4d9778e932ba Bug 725180 - Misc libmar fixes found from tests. http://hg.mozilla.org/mozilla-central/rev/2b1a53905350 Bug 725180 - Updater JS error handling for new error codes. http://hg.mozilla.org/mozilla-central/rev/5439f4751116 Bug 730862 - Disable signmar by default and provide an option to enable it. http://hg.mozilla.org/integration/mozilla-inbound/rev/5b9b207ac6ea Bug 730821 - Don't do security enhancements on x64 Windows builds. --- [Approval Request Comment] Regression caused by (bug #): These are security enhancements that should be included and landed from the service work in bug 481815 User impact if declined: Users may be subject to concerns from the security team Testing completed (on m-c, etc.): Everything except the last patch which is trivial and safe has been tested on m-c for several days. Risk to taking this patch (and alternatives if risky): There are some risks since it is over 300KB of changes, but it is a recommendation from the security team. String changes made by this patch: None
Attachment #601380 -
Flags: review?(robert.bugzilla)
Attachment #601380 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•12 years ago
|
Summary: Port security enhancements to Aurora → Port security enhancements for silent updates to Aurora
Assignee | ||
Comment 1•12 years ago
|
||
Added in these ones as well: https://hg.mozilla.org/mozilla-central/rev/1c3efb9a3fed Bug 725876 - Rename MOZ_UTILS_LDFLAGS to MOZ_GLUE_LDFLAGS and MOZ_UTILS_PROGRAM_LDFLAGS to MOZ_GLUE_PROGRAM_LDFLAGS https://hg.mozilla.org/mozilla-central/rev/7e3d5c2879a8 Bug 725568 - MozillaMaintenance service can crash after applying updates
Attachment #601380 -
Attachment is obsolete: true
Attachment #601398 -
Flags: review?(robert.bugzilla)
Attachment #601398 -
Flags: approval-mozilla-aurora?
Attachment #601380 -
Flags: review?(robert.bugzilla)
Attachment #601380 -
Flags: approval-mozilla-aurora?
Comment 2•12 years ago
|
||
Comment on attachment 601398 [details] [diff] [review] Patch v2. [Triage Comment] QA has signed off on this feature, it's a requirement for landing UAC for FF12, and we have a test day for silent updates this week which should shake out any regressions. Not to mention the fact that Aurora will update at least 14 times before it goes to Beta. Approving for Aurora 12. Please land ASAP (once we get r+'d on the rolled up patch if necessary).
Attachment #601398 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 3•12 years ago
|
||
Comment on attachment 601398 [details] [diff] [review] Patch v2. Please also port the fix to UpdateLog::Init in updatelogging.cpp and updatelogging.h
Comment 4•12 years ago
|
||
Specifically, removal of const from NS_tchar* fileName
Comment 5•12 years ago
|
||
Comment on attachment 601398 [details] [diff] [review] Patch v2. The only difference that isn't clear to me is the following (+ denotes existence on m-a and not m-c). --- ../../_1_mozilla-central/mozilla/toolkit/mozapps/update/updater/updater.cpp 2012-02-28 13:14:09 -0800 +++ toolkit/mozapps/update/updater/updater.cpp 2012-02-28 13:18:05 -0800 @@ -1457,17 +1457,16 @@ static const char kApplying[] = "applying"; if (fwrite(kApplying, strlen(kApplying), 1, file) != 1) return false; return true; } -#ifdef MOZ_MAINTENANCE_SERVICE /* * Read the update.status file and sets isPendingService to true if * the status is set to pending-service. * * @param isPendingService Out parameter for specifying if the status * is set to pending-service or not. * @return true if the information was retrieved and it is pending * or pending-service. @@ -1492,19 +1491,17 @@ const char kPendingService[] = "pending-service"; isPending = strncmp(buf, kPending, sizeof(kPending) - 1) == 0; isPendingService = strncmp(buf, kPendingService, sizeof(kPendingService) - 1) == 0; return isPending; } -#endif -#ifdef XP_WIN /* * Read the update.status file and sets isSuccess to true if * the status is set to succeeded. * * @param isSucceeded Out parameter for specifying if the status * is set to succeeded or not. * @return true if the information was retrieved and it is succeeded. */ @@ -1524,16 +1521,17 @@ fread(buf, sizeof(buf), 1, file); const char kSucceeded[] = "succeeded"; isSucceeded = strncmp(buf, kSucceeded, sizeof(kSucceeded) - 1) == 0; return true; } +#ifdef XP_WIN static void WaitForServiceFinishThread(void *param) { // We wait at most 10 minutes, we already waited 5 seconds previously // before deciding to show this UI. WaitForServiceStop(SVC_NAME, 595); LOG(("calling QuitProgressUI\n")); QuitProgressUI(); Please remove isComplete as well @@ -2705,32 +2614,34 @@ LOG(("DoUpdate: error opening manifest file: " LOG_S "\n", manifest)); return READ_ERROR; } ActionList list; NS_tchar *line; bool isFirstAction = true; + bool isComplete = false; while((line = mstrtok(kNL, &rb)) != 0) { // skip comments if (*line == NS_T('#')) continue; NS_tchar *token = mstrtok(kWhitespace, &line); if (!token) { LOG(("DoUpdate: token not found in manifest\n")); return PARSE_ERROR; } if (isFirstAction && NS_tstrcmp(token, NS_T("type")) == 0) { const NS_tchar *type = mstrtok(kQuote, &line); LOG(("UPDATE TYPE " LOG_S "\n", type)); if (NS_tstrcmp(type, NS_T("complete")) == 0) { + isComplete = true; rv = AddPreCompleteActions(&list); if (rv) return rv; } isFirstAction = false; continue; } r-ing mainly because of the first item above where it isn't clear why these ifdef's don't also exist on aurora (is it due to the x64 patch?).
Attachment #601398 -
Flags: review?(robert.bugzilla) → review-
Assignee | ||
Comment 6•12 years ago
|
||
> it isn't clear why these ifdef's don't also exist on aurora
>(is it due to the x64 patch?).
That change is because there's another compiler warning patch for non Windows platforms for unreferenced functions. I'll include that patch as well
Assignee | ||
Comment 7•12 years ago
|
||
Implicitly carrying forward the a+ since I can't mark an a+ myself. Turns out both of those warning changes are the same changeset: Bug 727156 - fix compiler warnings in toolkit/mozapps/update/ https://hg.mozilla.org/mozilla-central/rev/a8634484e86b
Attachment #601398 -
Attachment is obsolete: true
Attachment #601459 -
Flags: review?(robert.bugzilla)
Comment 8•12 years ago
|
||
Comment on attachment 601459 [details] [diff] [review] Patch v3. The interdiff between m-c and m-a with this patch looks good!
Attachment #601459 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 9•12 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/18840d884c97
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
status-firefox12:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•