#19410 closed defect (fixed)
Incremental updates from 6.0 to 6.0.1 are not working on OS X
Reported by: | gk | Owned by: | boklm |
---|---|---|---|
Priority: | High | Milestone: | |
Component: | Applications/Tor Browser | Version: | |
Severity: | Major | Keywords: | tbb-6.0-issues, TorBrowserTeam201609R |
Cc: | mcs, brade | Actual Points: | |
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
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.
Child Tickets
Attachments (1)
Change History (28)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
Keywords: | tbb-6.0-issues added |
---|
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.
comment:3 Changed 4 years ago by
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).
comment:4 Changed 4 years ago by
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. :(
comment:5 follow-ups: 6 9 Changed 4 years ago by
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).
comment:6 follow-up: 7 Changed 3 years ago by
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.
comment:7 follow-up: 8 Changed 3 years ago by
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.
comment:8 follow-up: 10 Changed 3 years ago by
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.
comment:9 follow-up: 11 Changed 3 years ago by
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?
comment:10 Changed 3 years ago by
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.
comment:11 Changed 3 years ago by
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....
comment:12 Changed 3 years ago by
Keywords: | TorBrowserTeam201607 added; TorBrowserTeam201606 removed |
---|---|
Owner: | changed from tbb-team to boklm |
Status: | new → assigned |
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.
comment:13 Changed 3 years ago by
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.
comment:14 Changed 3 years ago by
Keywords: | TorBrowserTeam201608R added; TorBrowserTeam201607 removed |
---|---|
Status: | assigned → needs_review |
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
comment:15 follow-up: 16 Changed 3 years ago by
mcs/brade could you take a look at boklm's approach?
comment:16 Changed 3 years ago by
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.
comment:17 follow-up: 18 Changed 3 years ago by
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.
comment:18 follow-up: 19 Changed 3 years ago by
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/).
comment:19 Changed 3 years ago by
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.
comment:20 Changed 3 years ago by
Keywords: | TorBrowserTeam201608 added; TorBrowserTeam201608R removed |
---|---|
Status: | needs_review → needs_revision |
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.
comment:21 Changed 3 years ago by
Keywords: | TorBrowserTeam201609 added; TorBrowserTeam201608 removed |
---|
Tickets for September.
comment:22 follow-up: 25 Changed 3 years ago by
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.
Changed 3 years ago by
Attachment: | 0001-Bug-19410-add-instructions-for-generating-code-signe.patch added |
---|
comment:23 Changed 3 years ago by
Keywords: | TorBrowserTeam201609R added; TorBrowserTeam201609 removed |
---|---|
Status: | needs_revision → needs_review |
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).
comment:24 Changed 3 years ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
Thanks, looks good to me. Applied to master with commit 23bea36eb8d8d27ca00b2394757be08530cb7f19 (tor-browser-bundle) and commit 4747b28324da7266b2e28cea5d771eb84bb3f1ce (tor-browser-spec).
comment:25 Changed 3 years ago by
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/
comment:26 Changed 3 years ago by
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.
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:
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: