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

[gk]

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.

[mcs]

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.

[gk]

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.

[mcs]

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

[gk]

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. :(

[mcs]

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

[boklm]

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.

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

[gk]

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.

[boklm]

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?

[mcs]

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.

[mcs]

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

[gk]

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.

[boklm]

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.

[boklm]

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

[gk]

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

[mcs]

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.

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

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

[boklm]

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/).

[mcs]

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.

[gk]

I think we could need an easy integration of this script into our build process as well, like a make target. Then, could you add a patch for our updated release procedures to this ticket, too?
While being a bit annoying the p7zip situation is not that dramatic. There won't be many that need to go through the hassle you are describing in your patch. FWIW: I think you should make the comment there future proof saying that Debian Stretch users can install this directly from the repo while users with older Debian versions and/or p7zip < 15.14 need to follow the things you are outlining in the comment. After all Debian Stretch is supposed to get stable at the end of the year IIRC.

I guess once this lands we need to adapt our explanation on how to verify the MAR files Gitian built are really the same as those we ship. :/ But this can be a different ticket.

[gk]

Tickets for September.

[boklm]

It seems 7zip does not correctly extract symlinks from dmg files (they are replaced by empty files). It is not a problem as we don't use any symlink currently (except the Applications -> /Applications, which is not included in the mar files), but it could be a problem if we start using symlinks in the future.

[boklm]

I attached a patch for the tor-browser-spec repo, adding instructions to regenerate the mar files.

I also pushed a bug_19410-v4 branch:
​https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/commit/?h=bug_19410-v4

This branch adds the dmg2mars and dmg2mars-alpha makefile rules. It also update the p7zip URLs in the comments in the dm2mar to use snapshot.debian.org (the previous URLs are no longer working).

[gk]

Thanks, looks good to me. Applied to master with commit 23bea36eb8d8d27ca00b2394757be08530cb7f19 (tor-browser-bundle) and commit 4747b28324da7266b2e28cea5d771eb84bb3f1ce (tor-browser-spec).

[boklm]

Replying to boklm:

It seems 7zip does not correctly extract symlinks from dmg files (they are replaced by empty files). It is not a problem as we don't use any symlink currently (except the Applications -> /Applications, which is not included in the mar files), but it could be a problem if we start using symlinks in the future.

Regarding this issue, I posted a message on their forum:
​https://sourceforge.net/p/sevenzip/discussion/45798/thread/3592e4b0/

[mcs]

Regarding the symlink issue, as a failsafe we could check to make sure no symlinks are used, but we would need to do that as we create the dmg.

[mcs]

I created #20135 for the idea I mentioned in comment:26.