Incremental updates from 6.0 to 6.0.1 are not working on OS X

I tried to update from 6.0 to 6.0.1 and first it downloads the incremental update. Verification is fine it seems but then I get the following error:

AUS:SVC Downloader:_verifyDownload called
AUS:SVC Downloader:_verifyDownload downloaded size == expected size.
AUS:SVC Downloader:_verifyDownload hashes match.
AUS:SVC Downloader:onStopRequest - setting state to: pending
AUS:SVC Downloader:onStopRequest - attempting to stage update: Tor Browser 6.0.1
AUS:SVC readStatusFile - status: failed: 2, path: /Users/release/Desktop/TorBrowser-Data/UpdateInfo/Users/release/Desktop/TorBrowser/updates/0/update.status
AUS:SVC handleFallbackToCompleteUpdate - install of partial patch failed, downloading complete patch
AUS:SVC Creating Downloader
AUS:SVC UpdateService:_downloadUpdate
AUS:SVC readStringFromFile - file doesn't exist: /Users/release/Desktop/TorBrowser-Data/UpdateInfo/Users/release/Desktop/TorBrowser/updates/0/update.status
AUS:SVC readStatusFile - status: null, path: /Users/release/Desktop/TorBrowser-Data/UpdateInfo/Users/release/Desktop/TorBrowser/updates/0/update.status
AUS:SVC Downloader:_selectPatch - found existing patch with state: null
AUS:SVC Downloader:downloadUpdate - downloading from https://www.torproject.org/dist/torbrowser/6.0.1/tor-browser-osx64-6.0.1_en-US.mar to /Users/release/Desktop/TorBrowser-Data/UpdateInfo/Users/release/Desktop/TorBrowser/updates/0/update.mar
AUS:SVC UpdateManager:refreshUpdateStatus - Notifying observers that the update was staged. state: downloading, status: failed: 2

And the browser is falling back to downloading and applying the full update successfully.

added TorBrowserTeam201609R component::applications/tor browser owner::boklm priority::high resolution::fixed severity::major status::closed tbb-6.0-issues type::defect labels

I am able to reproduce this problem. I am not sure why Kathy and I did not notice it when we updated from 6.0 to 6.0.1 on two different Mac OS systems, but starting with a clean install of 6.0 I see the same problem you did.

Looking at the last-update.log file (located deep under the UpdateInfo directory), I see why the incremental update failed to apply:

...
EXECUTE PATCH Contents/MacOS/updater.app/Contents/MacOS/updater
LoadSourceFile: destination file size 106848 does not match expected size 97000
LoadSourceFile failed
### execution failed

What this means is that the incremental MAR specifies that the size of the unpatched Contents/MacOS/updater.app/Contents/MacOS/updater (from 6.0) should be 97000 bytes but it is 106848 instead.

Two possibilities come to mind:

1. The incremental MAR was generated from the wrong starting version (but several files were patched successfully before the updater patch attempt failed, so that seems unlikely to me).
2. There is a bug in the code that generates the incremental MAR.

Just as additional data points:

1. The incremental update from 5.5.5 to 6.0.1 is working fine on OS X (those incrementals got generated during the same build that generated the 6.0 -> 6.0.1 incrementals).
2. The incremental updates from 6.0 to 6.0.1 are working fine on Linux and Windows for me.

Trac:
Keywords: N/A deleted, tbb-6.0-issues added

Is it possible that the 6.0 -> 6.0.1 MAR was generated using unsigned packages? The Gatekeeper signing adds a signature to each executable, which might explain the updater file size difference as well as why the 5.5.5 -> 6.0.1 incremental MAR is correct (well, it may be updating people to something that lacks signatures, but it will still run since they "blessed" their TB at some time in the past).

Ugh, yes, that is what happened. Even worse I have at the moment no clue at all how to fix that given our signing setup for OS X. :(

6.0a5 -> 6.5a1 updates are also affected of course. Should we modify the update manifests to not advertise the flawed incremental MARs? Or can we manually generate new MARs (including complete MARs that contain signed files)?

I guess the big challenge is to sign everything before we generate the complete MAR files (or we could possibly regenerate them / create them later in the process).

Replying to mcs:

Should we modify the update manifests to not advertise the flawed incremental MARs? Or can we manually generate new MARs (including complete MARs that contain signed files)?

Is it flawed for everybody? It is flawed for the people who installed 6.0 using the .dmg, but maybe not for the people who installed it by doing the 5.5.5 -> 6.0 update.

Replying to boklm:

Is it flawed for everybody? It is flawed for the people who installed 6.0 using the .dmg, but maybe not for the people who installed it by doing the 5.5.5 -> 6.0 update.

I think you are correct: the 5.5.5 -> 6.0 -> 6.0.1 path probably works with two incremental updates. The browser that the user ends up with will unfortunately be different than the 6.0.1 DMG one though.

Replying to mcs:

Replying to boklm:

Is it flawed for everybody? It is flawed for the people who installed 6.0 using the .dmg, but maybe not for the people who installed it by doing the 5.5.5 -> 6.0 update.

I think you are correct: the 5.5.5 -> 6.0 -> 6.0.1 path probably works with two incremental updates. The browser that the user ends up with will unfortunately be different than the 6.0.1 DMG one though.

But just for one release, right? As far as I understand this is happening: if a user downloads a .dmg with Tor Browser >= 6.x they'll get a full update the next time irrespective whether there is an incremental one available. Thereafter, all OS X user should be on the same page again (modulo those that downloaded a fresh .dmg in the meantime) and incremental updates should be working again, too. Even relocating of the bundle (+ the TorBrowser-Data dir) should still leave a functional Tor Browser.

Replying to mcs:

I guess the big challenge is to sign everything before we generate the complete MAR files (or we could possibly regenerate them / create them later in the process).

Currently the files included in the mar files are not signed. Does this cause problems with Gatekeeper, for the users who installed the new version using the update?

So it looks like we need to update our signature/release process to regenerate the OSX mar files after signing the bundles. Maybe we can make a script that takes a signed .dmg file and a non-signed mar file as inputs, and generate a new mar file containing the signed files from the dmg?

Replying to gk:

But just for one release, right? As far as I understand this is happening: if a user downloads a .dmg with Tor Browser >= 6.x they'll get a full update the next time irrespective whether there is an incremental one available. Thereafter, all OS X user should be on the same page again (modulo those that downloaded a fresh .dmg in the meantime) and incremental updates should be working again, too. Even relocating of the bundle (+ the TorBrowser-Data dir) should still leave a functional Tor Browser.

Yes, although it bothers me that users will lose the Gatekeeper signatures.

Replying to boklm:

Currently the files included in the mar files are not signed. Does this cause problems with Gatekeeper, for the users who installed the new version using the update?

I think the Gatekeeper check is not done again unless the files are moved to another computer (once an app is opened and the signature is found to be valid, it is "blessed" on that Mac OS system).

So it looks like we need to update our signature/release process to regenerate the OSX mar files after signing the bundles. Maybe we can make a script that takes a signed .dmg file and a non-signed mar file as inputs, and generate a new mar file containing the signed files from the dmg?

Currently, we run the make_full_update.sh command during the bundling phase (mac/gitian-bundle.yml). Can we instead extract the .app from the signed .dmg and run make_full_update.sh using the signed .app? I think this is basically what you are suggesting....

As a reminder: this probably impacts our guide to the advanced MAR file verification as well: stripping the signature alone would not be enough anymore to verify that the (OS X) MAR files we ship are the ones the Gitian build creates.

boklm: Could you try to come up with something? I am fine leaving the advanced verification problem for a different ticket for now if that helps speeding up work on this one.

Trac:
Keywords: TorBrowserTeam201606 deleted, TorBrowserTeam201607 added
Owner: tbb-team to boklm
Status: new to assigned

In my branch bug_19410-v1 I added a script to convert .dmg files to .mar files: https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19410-v1&id=b8e75ab53f955098e4256b445a762782e8b88064

It requires a recent version of p7zip to be able to extract the dmg files. Unfortunately the version in Debian Jessie is too old. I added instructions to build it from source as a comment in the script.

We should run this script after signing the .dmg files, and before signing the .mar files.

The step that is still missing is the removal of the OSX incremental mar files, to be able to regenerate them.

I added a new version of the patch in branch bug_19410-v2 where the OSX incremental mar files are removed at the end: https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19410-v2&id=0592f51ed2f1042d866c4aa33c6ab50b44c40a80

Trac:
Keywords: TorBrowserTeam201607 deleted, TorBrowserTeam201608R added
Status: assigned to needs_review

mcs/brade could you take a look at boklm's approach?

Replying to gk:

mcs/brade could you take a look at boklm's approach?

The approach and the script look OK (my Perl is rusty, but I trust boklm's Perl skills). Kathy and I will try to run the script tomorrow as a double check on things.

I tested the tools/dmg2mar program and it seems to work as intended. But note that LC_ALL=C should be set before running dmg2mar. This will ensure that that make_full_update.sh creates mar files where the add instructions, etc. are in the same order as in the original mar files. It may also be possible to modify dmg2mar to set LC_ALL=C before exec'ing make_full_update.sh.

Also, requiring a newer version of p7zip is inconvenient but I am not sure what we can do about it.

Replying to mcs:

I tested the tools/dmg2mar program and it seems to work as intended. But note that LC_ALL=C should be set before running dmg2mar. This will ensure that that make_full_update.sh creates mar files where the add instructions, etc. are in the same order as in the original mar files. It may also be possible to modify dmg2mar to set LC_ALL=C before exec'ing make_full_update.sh.

Good point. I pushed a new branch bug_19410-v3 where we set LC_ALL=C: https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19410-v3

Also, requiring a newer version of p7zip is inconvenient but I am not sure what we can do about it.

Yes, it is inconvenient. But I don't know any other tool available in Debian that can extract dmg files. Maybe we can help getting a backports package available in Debian to make installation easier (following the guidelines in https://backports.debian.org/Contribute/).

Replying to boklm:

Good point. I pushed a new branch bug_19410-v3 where we set LC_ALL=C: https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19410-v3

r=brade, r=mcs Looks good.