Closed
Bug 770996
Opened 12 years ago
Closed 12 years ago
partial mars broken for mac partner builds
Categories
(Release Engineering :: Release Automation: Other, defect)
Release Engineering
Release Automation: Other
Tracking
(firefox13 affected, firefox14+ fixed, firefox15+ affected, firefox16+ fixed, firefox17 fixed, firefox18 fixed)
People
(Reporter: bhearsum, Assigned: bhearsum)
References
Details
(Keywords: qawanted)
Attachments
(3 files)
(deleted),
patch
|
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
catlee
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
nthomas
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
nthomas
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
Because we're resigning mac partner repacks, codesign modifies the Contents/MacOS/firefox binary. Because of this, when they attempt to apply a partial update it will fail, and they will fallback to the complete. We purposely includeI don't know how we fix this without excluding distribution/, or creating special MARs for partner rd distribution/ in the signature because otherwise people could make any changes they want through addons, etc. and they signature would still be valid and claim to be from Mozilla. I don't know any good fix right now. We could:
- Create special MARs for partner repacks (expensive, painful, no scripts to do it at this time)
- Exclude distribution from the signature (lets people alter behaviour without invalidating the signature)
- Block partial updates for partner repacks (requires a bunch of custom snippet work)
- Do nothing (bad UX)
Assignee | ||
Comment 1•12 years ago
|
||
Let's try this again:
Because we're resigning mac partner repacks, codesign modifies the Contents/MacOS/firefox binary. Because of this, when they attempt to apply a partial update it will fail, and they will fallback to the complete. We purposely included distribution/ in the signature because otherwise people could make any changes they want through addons, etc. and the signature would still be valid and claim to be from Mozilla. I don't know any good fix right now. We could:
- Create special MARs for partner repacks (expensive, painful, no scripts to do it at this time)
- Exclude distribution from the signature (lets people alter behaviour without invalidating the signature)
- Block partial updates for partner repacks (requires a bunch of custom snippet work)
- Do nothing (bad UX)
Assignee | ||
Comment 2•12 years ago
|
||
Original bug is https://bugzilla.mozilla.org/show_bug.cgi?id=759318
Comment 3•12 years ago
|
||
Would it make any sense to change when we sign the app (post re-pack)?
tracking-firefox15:
--- → +
Assignee | ||
Comment 4•12 years ago
|
||
No, the problem is that there's a signature embedded within the 'firefox' binary (Contents/MacOS/firefox). That signature differs based on the contents of CodeResources (as best we can tell). Because distribution/ is part of the signature, CodeResources ends up with checksums in it - which will be different for every locale+partner/vanilla+platform combination.
Comment 5•12 years ago
|
||
Another issue is that the CodeResources file will (presumably) differ between vanilla and repacked builds, once bug 759328 comes into effect and distribution/* are included in it.
Assignee | ||
Comment 6•12 years ago
|
||
So, before go deep into OS X signing voodoo, is backing out bug 759318 a solution we should be considering?
Assignee | ||
Comment 7•12 years ago
|
||
Having bug 770995 would make it easier to have custom partials for these builds, but that bug is still in fantasy land with little chance of happening anytime soon.
Updated•12 years ago
|
status-firefox13:
--- → affected
status-firefox14:
--- → affected
status-firefox15:
--- → affected
tracking-firefox14:
--- → +
Comment 8•12 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #6)
> So, before go deep into OS X signing voodoo, is backing out bug 759318 a
> solution we should be considering?
We took bug 759318 mostly for correctness, since code signing was an install-driven feature as opposed to a security-driven feature. Backing out sounds like a good plan.
Assignee | ||
Comment 9•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #8)
> (In reply to Ben Hearsum [:bhearsum] from comment #6)
> > So, before go deep into OS X signing voodoo, is backing out bug 759318 a
> > solution we should be considering?
>
> We took bug 759318 mostly for correctness, since code signing was an
> install-driven feature as opposed to a security-driven feature. Backing out
> sounds like a good plan.
OK, great. I'll make this happen tomorrow.
Assignee | ||
Comment 10•12 years ago
|
||
Landed this on beta, aurora, and central:
https://hg.mozilla.org/releases/mozilla-beta/rev/c92edb5a0ac2
https://hg.mozilla.org/releases/mozilla-aurora/rev/226a2d776e84
https://hg.mozilla.org/mozilla-central/rev/faef72257478
Should be good to go here with 14.0 now.
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Assignee | ||
Comment 12•12 years ago
|
||
I'm not sure this is fixed, as it turns out. While investigating bug 787235 I did a comparison of a vanilla en-US 15.0 vs. a partner and found this:
bld-lion-r5-010:vanilla cltbld$ md5 Firefox.app/Contents/MacOS/firefox
MD5 (Firefox.app/Contents/MacOS/firefox) = 91e12442bcb3dadbcd37cfd89eb8f907
bld-lion-r5-010:partner cltbld$ md5 Firefox.app/Contents/MacOS/firefox
MD5 (Firefox.app/Contents/MacOS/firefox) = 9f03b204f94a4bc56f1a66998cba468e
Which means that partner builds of 15.0 that get a partial update to 16.0 will fail, and they won't fall back due to bug 774618.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 14•12 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #12)
> Which means that partner builds of 15.0 that get a partial update to 16.0
> will fail, and they won't fall back due to bug 774618.
Adding qawanted to confirm what the experience is when updating a FF14.0.1 partner build to FF15.0 (presumably bug 774618).
This seems pretty concerning - do you agree that our only option for updating FF14/15 partner build users may be to create custom partial updates to FF16? Then we'd presumably stop the bleeding in the future by fixing bug 774618 in FF16?
Comment 15•12 years ago
|
||
Urgh. I can confirm a en-US 14.0.1 funnelcake14 build fails the partial (this bug) then doesn't failover to the complete (bug 774618). It will affect about 90k ADI.
We can block 14.0.1 -> 15.0 partials for mac to work around this in the meantime. Bug 787863 for that.
Comment 16•12 years ago
|
||
What, if any, are the implications here that we should be prepared for in the case of a 15.0.1 chemspill?
Comment 17•12 years ago
|
||
(adding rstrong in case there are client side update issues, as opposed to just mar generation)
Comment 18•12 years ago
|
||
AIUI, mac signing will always modify the firefox binary with new signature information
nthomas suggested we always include the full firefox binary in the partial mar.
we can also look at disabling signing for partner repacks.
Comment 19•12 years ago
|
||
(In reply to Chris AtLee [:catlee] from comment #18)
> nthomas suggested we always include the full firefox binary in the partial
> mar.
This would be the Contents/MacOS/firefox executable, which is ~55KB and would have negligible impact on the partial mar size. The prefix should make it mac-specific.
IIRC we'd have to modify this line
http://hg.mozilla.org/releases/mozilla-release/file/default/tools/update-packaging/make_incremental_update.sh#l60
to do that, as we don't currently pass any forces in from buildbot.
AFAIK this wouldn't make the signing any less secure than attachment 641017 [details] [diff] [review] intended to leave us, ie we're not enforcing that the contents of distribution/ be unmodified either way, but I wouldn't claim to be well versed in this security realm.
Comment 20•12 years ago
|
||
Using this I recreated the 13.0.1 -> 15.0 partial mar for vanilla mac en-US, then unpacked the original mar to before/ and the new one to after/.
$ diff -ru before/ after/ | cut -c-80
Only in before/Contents/MacOS: firefox.patch
Only in after/Contents/MacOS: firefox
diff -ru before/update.manifest after/update.manifest
--- before/update.manifest 2012-09-04 15:39:12.000000000 -0700
+++ after/update.manifest 2012-09-04 15:37:12.000000000 -0700
@@ -36,7 +36,7 @@
patch "Contents/MacOS/libfreebl3.dylib.patch" "Contents/MacOS/libfreebl3.dylib"
add "Contents/MacOS/libfreebl3.chk"
patch "Contents/MacOS/firefox-bin.patch" "Contents/MacOS/firefox-bin"
-patch "Contents/MacOS/firefox.patch" "Contents/MacOS/firefox"
+add "Contents/MacOS/firefox"
patch-if "Contents/MacOS/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}" "Co
patch-if "Contents/MacOS/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}" "Co
patch "Contents/MacOS/dictionaries/en-US.aff.patch" "Contents/MacOS/dictionarie
diff -ru before/updatev2.manifest after/updatev2.manifest
--- before/updatev2.manifest 2012-09-04 15:39:13.000000000 -0700
+++ after/updatev2.manifest 2012-09-04 15:37:13.000000000 -0700
@@ -37,7 +37,7 @@
patch "Contents/MacOS/libfreebl3.dylib.patch" "Contents/MacOS/libfreebl3.dylib"
add "Contents/MacOS/libfreebl3.chk"
patch "Contents/MacOS/firefox-bin.patch" "Contents/MacOS/firefox-bin"
-patch "Contents/MacOS/firefox.patch" "Contents/MacOS/firefox"
+add "Contents/MacOS/firefox"
patch-if "Contents/MacOS/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}" "Co
patch-if "Contents/MacOS/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}" "Co
patch "Contents/MacOS/dictionaries/en-US.aff.patch" "Contents/MacOS/dictionarie
ie include the whole firefox file, and overwrite instead of patching in the manifests. Otherwise no change.
The file size increased 10882 bytes to 17776750 bytes.
Attachment #658280 -
Flags: review?(catlee)
Comment 21•12 years ago
|
||
Comment on attachment 658280 [details] [diff] [review]
Force Contents/MacOS/firefox in partials
The test mar I created successfully updated 13.0.1 funnelcake11 to 15.0. I think this is fairly safe but there may be some risk to put this straight into a chemspill. What do you think akeybl ?
Comment 22•12 years ago
|
||
(In reply to Nick Thomas [:nthomas] from comment #21)
> Comment on attachment 658280 [details] [diff] [review]
> Force Contents/MacOS/firefox in partials
>
> The test mar I created successfully updated 13.0.1 funnelcake11 to 15.0. I
> think this is fairly safe but there may be some risk to put this straight
> into a chemspill. What do you think akeybl ?
Is the alternative to provide complete mars for all partner build updates? Basically, I'd like to take the lowest risk solution that successfully moves our users to the latest version.
Comment 23•12 years ago
|
||
Lower touch than that, but still fairly involved. We'd need to
* enumerate the list of all the active partner channels (eg release-cck-yandex, release-cck-mozilla14 for funnelcake14)
* figure out the versions where we have signing on mac, and also offer a partial upgrade
* create empty snippets each pair of channel & version, which block falling back to the partial on the release channel (for all locales etc)
* test and publish
As new releases come along we'd need to add new empty snippets, so there's an ongoing cost. It would also slow down pushing new snippets out as we accumulate more snippets to backup/push.
In comparison changing the partial generation is a once-off change, albeit one with a larger scope than just the partners. From inspection of the mar generation code the patch should not put linux or windows at risk because they don't share the Contents/MacOS prefix.
Comment 24•12 years ago
|
||
Comment on attachment 658280 [details] [diff] [review]
Force Contents/MacOS/firefox in partials
Review of attachment 658280 [details] [diff] [review]:
-----------------------------------------------------------------
this looks safe to me
Attachment #658280 -
Flags: review?(catlee) → review+
Comment 25•12 years ago
|
||
Comment on attachment 658280 [details] [diff] [review]
Force Contents/MacOS/firefox in partials
http://hg.mozilla.org/integration/mozilla-inbound/rev/8943f92197c3
Attachment #658280 -
Flags: checked-in+
Updated•12 years ago
|
Whiteboard: [leave open]
Comment 26•12 years ago
|
||
Comment 27•12 years ago
|
||
My funnelcake 14 that was unable to update automatically updated to 15.0.1 after I restarted the browser today. Last week, this was not the case and the update would not apply.
Assignee | ||
Comment 28•12 years ago
|
||
(In reply to Chris More [:cmore] from comment #27)
> My funnelcake 14 that was unable to update automatically updated to 15.0.1
> after I restarted the browser today. Last week, this was not the case and
> the update would not apply.
The work that was done in this bug (in particular, attachment #658280 [details] [diff] [review]) fixed that for 15.0.1. Thanks for confirming!
Comment 29•12 years ago
|
||
Comment on attachment 658280 [details] [diff] [review]
Force Contents/MacOS/firefox in partials
This is working as expected in Nightly builds, eg from the last-update.log:
EXECUTE ADD Contents/MacOS/firefox
...
FINISH ADD Contents/MacOS/firefox
...
succeeded
calling QuitProgressUI
[Approval Request Comment]
Bug caused by (feature/regressing bug #): mac signing in general (bug 730924) and bug 759318 (sign partner builds harder to avoid malware)
User impact if declined: 13.0 through 15.0.1 mac partner users will have a bad update experience
Testing completed (on m-c, etc.): baked on Nightly for 3 updates
Risk to taking this patch (and alternatives if risky): low risk, this merely forces an update for a file we would have patched
String or UUID changes made by this patch: none
Attachment #658280 -
Flags: approval-mozilla-beta?
Attachment #658280 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 30•12 years ago
|
||
Nick, here's the other approach you and I talked about a bit in e-mail, and it was also brought up in the 15.0 post mortem today and nobody had objections to it.
To check that this won't have negative side-effects I diff'ed a partner build vs. its vanilla one, and found the only differences to be the existence of distribution/, and the firefox binary itself. The distribution/ difference is expected and doesn't break the signature, and with this patch the firefox binary will end up the same again, fixing the partials and removing the need to force a full update for the firefox binary.
Attachment #660161 -
Flags: review?(nthomas)
Comment 31•12 years ago
|
||
> The work that was done in this bug (in particular, attachment #658280 [details] [diff] [review]
> fixed that for 15.0.1. Thanks for confirming!
Just to clarify, we shouldn't be concerned about people being stuck on fx 14 mac partner builds (without having to manually upgrade) going forward after they restart their browser?
Assignee | ||
Comment 32•12 years ago
|
||
(In reply to Chris More [:cmore] from comment #31)
>
> > The work that was done in this bug (in particular, attachment #658280 [details] [diff] [review] [diff] [review]
> > fixed that for 15.0.1. Thanks for confirming!
>
> Just to clarify, we shouldn't be concerned about people being stuck on fx 14
> mac partner builds (without having to manually upgrade) going forward after
> they restart their browser?
Right now, partials for partner users on Mac are failing due to this bug. I would expect the fallback to a complete not to happen either due to bug 774618, but your experience shows this isn't always the case. This bug will be fixed for the next release though, so we don't have to worry about people being stranded due to this at that point.
Comment 33•12 years ago
|
||
Comment on attachment 658280 [details] [diff] [review]
Force Contents/MacOS/firefox in partials
[Triage Comment]
Approving in support of working updates for partner repacks in future releases.
Attachment #658280 -
Flags: approval-mozilla-beta?
Attachment #658280 -
Flags: approval-mozilla-beta+
Attachment #658280 -
Flags: approval-mozilla-aurora?
Attachment #658280 -
Flags: approval-mozilla-aurora+
Updated•12 years ago
|
Attachment #660161 -
Flags: review?(nthomas) → review+
Comment 34•12 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #32)
> (In reply to Chris More [:cmore] from comment #31)
> >
> > > The work that was done in this bug (in particular, attachment #658280 [details] [diff] [review] [diff] [review] [diff] [review]
> > > fixed that for 15.0.1. Thanks for confirming!
We haven't actually built any release mar files with this patch yet, instead we've suppressed partial updates for mac partner builds (in the update server). The remaining complete updates replace the firefox executable rather than patching so the update works ok, but at the cost of a much larger download.
> > Just to clarify, we shouldn't be concerned about people being stuck on fx 14
> > mac partner builds (without having to manually upgrade) going forward after
> > they restart their browser?
>
> Right now, partials for partner users on Mac are failing due to this bug. I
> would expect the fallback to a complete not to happen either due to bug
> 774618, but your experience shows this isn't always the case. This bug will
> be fixed for the next release though, so we don't have to worry about people
> being stranded due to this at that point.
I think Chris got a complete update, about 32MB instead of 14MB right ? You could check by looking at Firefox.app/Contents/MacOS/updates.xml.
At any rate for 16.0 we'll probably have a complete from 13.0 (when signing started), and partials that force firefox for anything more recent than that. Plus we'll stop re-signing mac partner builds then, so the underlying issue will go away.
Comment 35•12 years ago
|
||
Here is my updates.xml:
<updates xmlns="http://www.mozilla.org/2005/app-update"><update appVersion="undefined" buildID="undefined" channel="default" displayVersion="undefined" extensionVersion="undefined" installDate="undefined" isCompleteUpdate="false" name="undefined" serviceURL="undefined" showNeverForVersion="false" showPrompt="false" showSurvey="false" type="undefined" version="undefined" detailsURL="http://www.mozilla.com/en-US/firefox/releases/" statusText="The Update was successfully installed"/></updates>
Comment 36•12 years ago
|
||
Comment on attachment 658280 [details] [diff] [review]
Force Contents/MacOS/firefox in partials
https://hg.mozilla.org/releases/mozilla-aurora/rev/8ad864f65eb7
https://hg.mozilla.org/releases/mozilla-beta/rev/b4dd8cdec0ad
akeybl, I'd like to make sure this patch is included in any 15.0.2 chemspill, but understand there is no certainly about if we are doing one. Should I go ahead and request approval-mozilla-release so that we don't miss it in a rush ?
Assignee | ||
Comment 37•12 years ago
|
||
Comment on attachment 660161 [details] [diff] [review]
rip out mac signing from partner repack scripts
With this landed, any future releases we do will have a consistent 'firefox' binary between vanilla and partner repacks.
Attachment #660161 -
Flags: checked-in+
Updated•12 years ago
|
Assignee | ||
Comment 38•12 years ago
|
||
Modulo bug 805788, which tracks _another_ addition to files excluded from the signature, we're all done here AFAICT.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•