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)

defect
Not set
normal

Tracking

(firefox13 affected, firefox14+ fixed, firefox15+ affected, firefox16+ fixed, firefox17 fixed, firefox18 fixed)

RESOLVED FIXED
Tracking Status
firefox13 --- affected
firefox14 + fixed
firefox15 + affected
firefox16 + fixed
firefox17 --- fixed
firefox18 --- fixed

People

(Reporter: bhearsum, Assigned: bhearsum)

References

Details

(Keywords: qawanted)

Attachments

(3 files)

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)
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)
Would it make any sense to change when we sign the app (post re-pack)?
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.
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.
So, before go deep into OS X signing voodoo, is backing out bug 759318 a solution we should be considering?
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.
(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.
(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.
Attached patch partial backout of bug 759318 (deleted) — Splinter Review
Assignee: nobody → bhearsum
Status: NEW → ASSIGNED
Attachment #641017 - Flags: checked-in+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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 → ---
(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?
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.
Depends on: 787863
What, if any, are the implications here that we should be prepared for in the case of a 15.0.1 chemspill?
(adding rstrong in case there are client side update issues, as opposed to just mar generation)
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.
(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.
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 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 ?
(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.
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 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+
Whiteboard: [leave open]
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.
(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 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?
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)
> 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?
(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 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+
Attachment #660161 - Flags: review?(nthomas) → review+
(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.
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 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 ?
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+
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 ago12 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: