partial mars broken for mac partner builds

RESOLVED FIXED

Status

Release Engineering
Release Automation
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: bhearsum, Assigned: bhearsum)

Tracking

({qawanted})

other
qawanted

Firefox Tracking Flags

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

Details

Attachments

(3 attachments)

(Assignee)

Description

5 years ago
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

5 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

5 years ago
Original bug is https://bugzilla.mozilla.org/show_bug.cgi?id=759318

Comment 3

5 years ago
Would it make any sense to change when we sign the app (post re-pack)?
tracking-firefox15: --- → +
(Assignee)

Comment 4

5 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.
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

5 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

5 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

5 years ago
status-firefox13: --- → affected
status-firefox14: --- → affected
status-firefox15: --- → affected
tracking-firefox14: --- → +

Comment 8

5 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

5 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

5 years ago
Created attachment 641017 [details] [diff] [review]
partial backout of bug 759318

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: nobody → bhearsum
Status: NEW → ASSIGNED
Attachment #641017 - Flags: checked-in+
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
status-firefox14: affected → fixed
status-firefox15: affected → fixed
(Assignee)

Updated

5 years ago
Duplicate of this bug: 774618
(Assignee)

Comment 12

5 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 → ---
(Assignee)

Updated

5 years ago
Duplicate of this bug: 787235
(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?
status-firefox15: fixed → affected
tracking-firefox16: --- → +
Keywords: qawanted
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.

Updated

5 years ago
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.
Created attachment 658280 [details] [diff] [review]
Force Contents/MacOS/firefox in partials

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+
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

5 years ago
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/8943f92197c3

Comment 27

5 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

5 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 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

5 years ago
Created attachment 660161 [details] [diff] [review]
rip out mac signing from partner repack scripts

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

5 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

5 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 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

5 years ago
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.

Comment 35

5 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 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

5 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

5 years ago
status-firefox16: --- → fixed
status-firefox17: --- → fixed
status-firefox18: --- → fixed
(Assignee)

Comment 38

5 years ago
Modulo bug 805788, which tracks _another_ addition to files excluded from the signature, we're all done here AFAICT.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 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.