Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#20691 closed defect (fixed)

Updates are not getting properly applied when trying to update to 6.5a4(-hardened) on Linux

Reported by: gk Owned by: tbb-team
Priority: Immediate Milestone:
Component: Applications/Tor Browser Version:
Severity: Critical Keywords: TorBrowserTeam201611R
Cc: mcs, brade, boklm, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

After turning on automatic updates for our alpha and hardened series it turned out the updates were not applied, at least not on Linux machines. They are therefore disabled at the moment which should get fixed as soon as possible.

Child Tickets

Attachments (1)

billboard.png (34.2 KB) - added by mcs 3 years ago.
billboard sample

Download all attachments as: .zip

Change History (35)

comment:1 Changed 3 years ago by gk

That's what I get with a test bundle

AUS:SVC Downloader:onStopRequest - status: 0, current fail: 0, max fail:
10, retryTimeout: 2000
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.5a4
AUS:SVC readStatusFile - status: failed: 6, path:
/home/thomas/Arbeit/Tor/tor-browser-bundle/tor-browser_de/Browser/TorBrowser/UpdateInfo/updates/0/update.status
AUS:SVC handleFallbackToCompleteUpdate - install of partial patch
failed, downloading complete patch

IIRC the complete update showed the same error message.

comment:2 Changed 3 years ago by mcs

I found signed mar files here: https://people.torproject.org/~gk/builds/6.5a4/ and was able to reproduce this problem by hosting an incremental mar file on my own server. My last-update.log file contained the following:

Performing a staged update
PATCH DIRECTORY /home/brade/Desktop/tb-test/Browser/TorBrowser/UpdateInfo/updates/0
INSTALLATION DIRECTORY /home/brade/Desktop/tb-test/Browser
WORKING DIRECTORY /home/brade/Desktop/tb-test/Browser/updated
ensure_copy: failed to open the file for reading: /home/brade/Desktop/tb-test/Browser/TorBrowser/Data/Tor/control.socket, err: 6
failed: 6
calling QuitProgressUI

It seems that the updater code is not smart enough to skip Unix domain sockets when copying the installation directory during the staged update process. I was able to update successfully on a 64-bit Linux system after I used about:config to set app.update.staging.enabled to false. The bad news is that as far as I know we cannot disable staged updates from the update manifest (server-side XML). Using the full mar file will result in the same problem as gk experienced.

I will try to think of a workaround but at the moment I do not have any ideas. I will ask Kathy about this problem in the morning.

A small bit of good news: I did not do any testing, but this problem should not occur on Windows (no Unix domain sockets and no staged updates either).

On OSX, I am able to successfully apply the update (the control port Unix domain socket is in the TorBrowser-Data directory, which is outside the scope of what the updater touches, so the ensure_copy problem does not occur). But on the OSX system that I used for testing, after the update was applied and the browser restarted, tor failed to start. This happened because quotes are missing from the ControlPort unix:... argument which was passed on the tor command line by Tor Launcher (I have spaces in my path). But we fixed that problem in Tor Launcher! After a little more investigation I learned that Tor Launcher was not updated in the profile or in the TorBrowser.app/Contents/Resources/distribution/extensions directory. I don't know why yet though, and we might need a new ticket for OSX. The dmg files have the correct Tor Launcher. I have not checked the complete mar files and I have not done enough analysis to determine if the incremental mar files are flawed or if the updater failed to update the xpi properly.

comment:3 in reply to:  2 Changed 3 years ago by gk

Replying to mcs:

I found signed mar files here: https://people.torproject.org/~gk/builds/6.5a4/ and was able to reproduce this problem by hosting an incremental mar file on my own server. My last-update.log file contained the following:

Performing a staged update
PATCH DIRECTORY /home/brade/Desktop/tb-test/Browser/TorBrowser/UpdateInfo/updates/0
INSTALLATION DIRECTORY /home/brade/Desktop/tb-test/Browser
WORKING DIRECTORY /home/brade/Desktop/tb-test/Browser/updated
ensure_copy: failed to open the file for reading: /home/brade/Desktop/tb-test/Browser/TorBrowser/Data/Tor/control.socket, err: 6
failed: 6
calling QuitProgressUI

It seems that the updater code is not smart enough to skip Unix domain sockets when copying the installation directory during the staged update process. I was able to update successfully on a 64-bit Linux system after I used about:config to set app.update.staging.enabled to false. The bad news is that as far as I know we cannot disable staged updates from the update manifest (server-side XML). Using the full mar file will result in the same problem as gk experienced.

I will try to think of a workaround but at the moment I do not have any ideas. I will ask Kathy about this problem in the morning.

A small bit of good news: I did not do any testing, but this problem should not occur on Windows (no Unix domain sockets and no staged updates either).

Thanks for the whole analysis so far! Just to reply to this bit: you are right. boklm and I tested updating to 6.5a4 on Windows and it worked both by using the partial and the full .mar. We have therefore enabled auto-updating for Windows users again.

comment:4 Changed 3 years ago by gk

Summary: Alpha updates are not getting applied when trying to update to 6.5a4(-hardened)Updates are not getting properly applied when trying to update to 6.5a4(-hardened) on OS X and Linux

comment:5 in reply to:  2 ; Changed 3 years ago by gk

Replying to mcs:

On OSX, I am able to successfully apply the update (the control port Unix domain socket is in the TorBrowser-Data directory, which is outside the scope of what the updater touches, so the ensure_copy problem does not occur). But on the OSX system that I used for testing, after the update was applied and the browser restarted, tor failed to start. This happened because quotes are missing from the ControlPort unix:... argument which was passed on the tor command line by Tor Launcher (I have spaces in my path). But we fixed that problem in Tor Launcher! After a little more investigation I learned that Tor Launcher was not updated in the profile or in the TorBrowser.app/Contents/Resources/distribution/extensions directory. I don't know why yet though, and we might need a new ticket for OSX. The dmg files have the correct Tor Launcher. I have not checked the complete mar files and I have not done enough analysis to determine if the incremental mar files are flawed or if the updater failed to update the xpi properly.

Interesting. I made a couple of tests taking a fresh 6.0.5 and a fresh 6.5a3. I tested updating the stable both in a desktop and an /Applications location and it worked: the extensions and everything seemed to be fine. Note, though, that we only have full updates right now for the stable series.

For the alpha I tested just installing it on the desktop and applying the MAR files manually. Both the incremental and the full MAR file applied correctly and the extensions were updated, too.

comment:6 in reply to:  5 ; Changed 3 years ago by gk

Replying to gk:

Replying to mcs:

On OSX, I am able to successfully apply the update (the control port Unix domain socket is in the TorBrowser-Data directory, which is outside the scope of what the updater touches, so the ensure_copy problem does not occur). But on the OSX system that I used for testing, after the update was applied and the browser restarted, tor failed to start. This happened because quotes are missing from the ControlPort unix:... argument which was passed on the tor command line by Tor Launcher (I have spaces in my path). But we fixed that problem in Tor Launcher! After a little more investigation I learned that Tor Launcher was not updated in the profile or in the TorBrowser.app/Contents/Resources/distribution/extensions directory. I don't know why yet though, and we might need a new ticket for OSX. The dmg files have the correct Tor Launcher. I have not checked the complete mar files and I have not done enough analysis to determine if the incremental mar files are flawed or if the updater failed to update the xpi properly.

Interesting. I made a couple of tests taking a fresh 6.0.5 and a fresh 6.5a3. I tested updating the stable both in a desktop and an /Applications location and it worked: the extensions and everything seemed to be fine. Note, though, that we only have full updates right now for the stable series.

For the alpha I tested just installing it on the desktop and applying the MAR files manually. Both the incremental and the full MAR file applied correctly and the extensions were updated, too.

I tested now putting 6.5a3 to /Applications (getting a space in the user dir path) and applying the incremental update manually worked as well. So, hm.

comment:7 in reply to:  6 ; Changed 3 years ago by mcs

Replying to gk:

For the alpha I tested just installing it on the desktop and applying the MAR files manually. Both the incremental and the full MAR file applied correctly and the extensions were updated, too.

I tested now putting 6.5a3 to /Applications (getting a space in the user dir path) and applying the incremental update manually worked as well. So, hm.

This morning I tried updating an existing/older 6.5a3 installation on another OSX system and it worked fine. Based on this and the fact that all of your tests worked, I think the OSX updates are probably okay. Kathy and I will try again on the OSX system that failed and let you know what we find, but I think you should re-enable updates for OSX.

Last edited 3 years ago by mcs (previous) (diff)

comment:8 in reply to:  7 Changed 3 years ago by mcs

Replying to mcs:

Kathy and I will try again on the OSX system that failed and let you know what we find, but I think you should re-enable updates for OSX.

It turns out that problem was a false alarm. The alpha install I tested with was an older one, and last night it updated from 6.5a2 to 6.5a3 (not a4), which led to the "oh no, things do not work due to a space in the path" behavior. So, never mind.

comment:9 Changed 3 years ago by gk

Summary: Updates are not getting properly applied when trying to update to 6.5a4(-hardened) on OS X and LinuxUpdates are not getting properly applied when trying to update to 6.5a4(-hardened) on Linux

comment:10 Changed 3 years ago by mcs

It is possible that we missed something, but after reading the updater and update service code, Kathy and I have concluded that the only workarounds are:

  • Users can set app.update.staging.enabled to false before attempting the update.
  • Users can disable the control port Unix domain socket by setting extensions.torlauncher.control_port_use_socket to false and restarting their browser before attempting the update.

The other thing to think about is "what will happen during the next update, i.e., 6.5a4 to 6.5a5?" The answer is that updates may fail (since the 6.5a4 updater which we already shipped is flawed in the same way as the 6.5a3 one). There is some good news though: because of the fix for #20185, users who have XDG_RUNTIME_DIR set (most people?) will not encounter this bug because the Unix domain sockets will be outside of the TB installation directory.

If XDG_RUNTIME_DIR is not set, similar workarounds will be needed for the 6.5a4 to 6.5a5 update. Note that the "disable Unix domain socket" prefs in 6.5a4 are extensions.torlauncher.control_port_use_ipc and extensions.torlauncher.socks_port_use_ipc (both would need to be set to false).

There are also prefs that control the location of the Unix domain sockets; these could be used to ensure that the sockets are created somewhere outside of the installation directory.

comment:11 Changed 3 years ago by mcs

Also, I thought we had an open ticket for "create an automated test for the updater" but I cannot find it at the moment.

comment:12 in reply to:  11 Changed 3 years ago by gk

Replying to mcs:

Also, I thought we had an open ticket for "create an automated test for the updater" but I cannot find it at the moment.

#17662.

comment:13 in reply to:  10 ; Changed 3 years ago by gk

Replying to mcs:

It is possible that we missed something, but after reading the updater and update service code, Kathy and I have concluded that the only workarounds are:

  • Users can set app.update.staging.enabled to false before attempting the update.
  • Users can disable the control port Unix domain socket by setting extensions.torlauncher.control_port_use_socket to false and restarting their browser before attempting the update.

The other thing to think about is "what will happen during the next update, i.e., 6.5a4 to 6.5a5?" The answer is that updates may fail (since the 6.5a4 updater which we already shipped is flawed in the same way as the 6.5a3 one). There is some good news though: because of the fix for #20185, users who have XDG_RUNTIME_DIR set (most people?) will not encounter this bug because the Unix domain sockets will be outside of the TB installation directory.

If XDG_RUNTIME_DIR is not set, similar workarounds will be needed for the 6.5a4 to 6.5a5 update. Note that the "disable Unix domain socket" prefs in 6.5a4 are extensions.torlauncher.control_port_use_ipc and extensions.torlauncher.socks_port_use_ipc (both would need to be set to false).

There are also prefs that control the location of the Unix domain sockets; these could be used to ensure that the sockets are created somewhere outside of the installation directory.

Thanks for looking into it. So, what do you suggest? Enabling the updates for Linux as well and doing a final update to our blog post? It seems in the worst case users are downloading the update again and again until they get the "Update failed x times" dialog. Can we say something sensibly on it conveying the current issue (with some attribute in the XML files)? I guess not? Or would we just leave the updater disabled and getting the onion to flash getting users to find the "Check for Tor Browser Update..." in the Torbutton menu? I guess this is suboptimal, at least for the reason that it is probably an unusal update flow even for alpha users.

comment:14 in reply to:  13 ; Changed 3 years ago by mcs

Replying to gk:

Thanks for looking into it. So, what do you suggest? Enabling the updates for Linux as well and doing a final update to our blog post?

Yes, because we do want users to get the update.

It seems in the worst case users are downloading the update again and again until they get the "Update failed x times" dialog. Can we say something sensibly on it conveying the current issue (with some attribute in the XML files)? I guess not? Or would we just leave the updater disabled and getting the onion to flash getting users to find the "Check for Tor Browser Update..." in the Torbutton menu? I guess this is suboptimal, at least for the reason that it is probably an unusal update flow even for alpha users.

I don't think we can add text to that dialog, but there is a "billboard" feature that we have never used that might be helpful in this case. (note that Mozilla recently remove that feature; see https://bugzilla.mozilla.org/show_bug.cgi?id=1296207). I tried it and it seems to work. You can create an HTML page that is displayed instead of the standard update prompt, and there is also a way to force the prompt to be shown even for automatic/background updates. Here is a sample update manifest; note the showPrompt and billboard attributes:

<?xml version="1.0"?>
<updates>
    <update type="minor" displayVersion="6.5a4" appVersion="6.5a4"
            platformVersion="45.5.0"
            buildID="20000101000000"
            showPrompt="true"
            billboardURL="https://example.com/tor/update_2/alpha/billboard.html"
            detailsURL="https://example.com/tor/details.html">
        <patch type="complete"
               URL="http://example.com/tor/update_2/alpha/mars/tor-browser-linux64-6.5a4_en-US.mar"
               hashFunction="SHA512"
               hashValue="ce9e233cac4495b803b08243603ccaf3c968c0ecb48f725e9ccb1a2b9d61c0b78cd71ecd324ed3b1a71875a94ecaeff0018ad059c139eaf61f37ad432fe8befe"
               size="89297856"/>
    </update>
</updates>

The other thing that took Kathy and me some time to figure out is that the HTML file must include a billboard attribute on the body, e.g.,

<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>Billboard Test</title>
</head>
<body billboard="true">
This is a test.
<p>
Useful info should be placed here.
</body>
</html> 

I will attach a screenshot that shows what this looks like. The text should include "Tor Browser 6.5a4 is available" plus whatever you include in the blog to help people work around this update issue (which most people will be affected by).

Changed 3 years ago by mcs

Attachment: billboard.png added

billboard sample

comment:15 Changed 3 years ago by mcs

Billboard sample:

billboard sample

comment:16 Changed 3 years ago by gk

Thanks. I guess we could try that billboard feature. But I won't have the energy to work on that anymore today. boklm: If you have, feel free to do so. I guess testing with the hardened series first is the way to go.

comment:17 Changed 3 years ago by mcs

Keywords: TorBrowserTeam201611R added; TorBrowserTeam201611 removed
Status: newneeds_review

comment:18 in reply to:  14 Changed 3 years ago by boklm

Replying to mcs:

I will attach a screenshot that shows what this looks like. The text should include "Tor Browser 6.5a4 is available" plus whatever you include in the blog to help people work around this update issue (which most people will be affected by).

Here is some text which we can include in the blog post and billboard page:

A new alpha version of Tor Browser is available. However, due to a bug in the
updater, the normal update process will fail for Linux users in the alpha
series. You can install the new Tor Browser alpha version after manually
downloading it from the Tor Project web site.

As an alternative, you can use one of the following workarounds to avoid the
updater error:
- in about:config, set app.update.staging.enabled to false before attempting
  to update
- in about:config, set extensions.torlauncher.control_port_use_socket to false
  (disabling the control port Unix domain socket) and restart the browser before
  attempting to update

Does it look correct?

comment:19 Changed 3 years ago by mcs

The text looks correct to me. I might replace "will fail" with "will probably fail" (if someone has already disabled the control port socket it will succeed). But either way is fine.

comment:20 in reply to:  19 Changed 3 years ago by boklm

Replying to mcs:

The text looks correct to me. I might replace "will fail" with "will probably fail" (if someone has already disabled the control port socket it will succeed). But either way is fine.

Thanks. I changed that and put the text in a web page:
https://aus1.torproject.org/torbrowser/update_2/bug_20691.html

I will now try to enable the updates with the billboard feature for hardened users.

comment:21 Changed 3 years ago by boklm

The update with the billboard is now enabled for hardened users, and the blog post updated:
https://blog.torproject.org/blog/tor-browser-65a4-hardened-released

I am going to do the same for the alpha users.

comment:22 Changed 3 years ago by boklm

This is now enabled for alpha users too.

comment:23 in reply to:  17 ; Changed 3 years ago by gk

Replying to mcs:

Kathy and I created a patch for the underlying updater bug. Please review.
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug20691-01&id=d2225a513147e33a9d50476174ca053cb9107880

Do we have an upstream bug for that problem already?

comment:24 in reply to:  23 ; Changed 3 years ago by mcs

Replying to gk:

Replying to mcs:

Kathy and I created a patch for the underlying updater bug. Please review.
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug20691-01&id=d2225a513147e33a9d50476174ca053cb9107880

Do we have an upstream bug for that problem already?

No, we did not create one. We will.
My guess is that Mozilla will never need this patch since they will never embed the user profile within the scope of what is being updated. It is a trivial patch though, so they might accept it.

comment:26 in reply to:  24 Changed 3 years ago by gk

Replying to mcs:

Replying to gk:

Replying to mcs:

Kathy and I created a patch for the underlying updater bug. Please review.
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug20691-01&id=d2225a513147e33a9d50476174ca053cb9107880

Do we have an upstream bug for that problem already?

No, we did not create one. We will.
My guess is that Mozilla will never need this patch since they will never embed the user profile within the scope of what is being updated. It is a trivial patch though, so they might accept it.

The user profile hint was a good one and I should have thought of it myself. After thinking a bit over the weekend I am with rstrong here and we should not uplift that one. Sorry for the extra work.

comment:27 in reply to:  17 ; Changed 3 years ago by gk

Cc: arthurdelstein added

Replying to mcs:

Kathy and I created a patch for the underlying updater bug. Please review.
https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug20691-01&id=d2225a513147e33a9d50476174ca053cb9107880

Looks good to me (I did not test the patch, though, assuming you did :) ).

comment:28 Changed 3 years ago by gk

Cc: arthuredelstein added; arthurdelstein removed

comment:29 in reply to:  27 Changed 3 years ago by mcs

Replying to gk:

Looks good to me (I did not test the patch, though, assuming you did :) ).

Yes we did.

comment:30 Changed 3 years ago by gk

mcs/brade: Should we use the, well, opportunity, to get that patch into the chemspill release?

comment:31 in reply to:  30 Changed 3 years ago by mcs

Replying to gk:

mcs/brade: Should we use the, well, opportunity, to get that patch into the chemspill release?

Yes please.

comment:32 Changed 3 years ago by gk

Arthur could you give that a quick second look? We'd like to ship it in 6.5a5.

comment:33 in reply to:  32 Changed 3 years ago by arthuredelstein

Replying to gk:

Arthur could you give that a quick second look? We'd like to ship it in 6.5a5.

Looks good to me.

comment:34 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks. This is commit fecc8672b045889a4a0560ff4e69fe5fc0fcefd7 on tor-browser-45.5.1esr-6.5-1 and will be available in 6.5a5 and 6.5a5-hardened.

Last edited 3 years ago by gk (previous) (diff)
Note: See TracTickets for help on using tickets.