#20778 closed enhancement (fixed)

Check updates in the background.

Reported by: yawning Owned by: yawning
Priority: High Milestone:
Component: Archived/Tor Browser Sandbox Version:
Severity: Normal Keywords: Yawning201612, sandbox-update
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by yawning)

Instead of the current logic which is "Check for updates once at launch, if it has been either over 6 hours since the last successful check OR the time of last check is in the future.", the update check/update download should happen every six hours in the background, and the user should be prompted to restart.

Full integration into the Tor Browser UI is likely a lot of work, so this is probably best accomplished by the agent popping up a dialog box when the update is complete (or using GNotification ~and a system tray icon/menu~).

Edit: Due to time constraints this ticket primarily concerns background update checks.

Child Tickets

Change History (20)

comment:1 Changed 19 months ago by yawning

If I do this, I can probably have Tor Browser (well torbutton)'s onion menu "Check for Tor Browser Update..." actually do something as well, by having it poke the sandbox agent to schedule an update check instead of what it usually does (eg: If an env var is set, send "FETCHUPDATE\r\n" over the control port, which is the sandboxed-tor-browser surrogate).

comment:2 Changed 19 months ago by yawning

Priority: MediumHigh

Bump up the priority on things I think are important for 0.0.2.

comment:3 Changed 18 months ago by yawning

Keywords: Yawning201612 added
Status: newaccepted

Tagging these as the things I will be focusing on for the rest of the year.

comment:4 Changed 18 months ago by yawning

Description: modified (diff)

comment:5 Changed 18 months ago by arma

I think Tor Browser does its checks every 12 hours? It would be nice to match their schedule, since I think the metrics folks have been planning to estimate extant Tor Browser users by counting update checks and dividing by n.

comment:6 Changed 18 months ago by yawning

https://git.schwanenlied.me/yawning/sandboxed-tor-browser/src/bug20778

Is the work in progress branch with a working background update check and #20851 fixed. It is not merged because it is lacking any sort of UI at all, except for "it dumps information to the log", and also does not handle downloading the MAR in the background yet.

If I can't figure out a good UI for all this, I might just opt for "display a dialog box if the browser is out of date" instead of "download the MAR in the background/integrate into the onion menu".

comment:7 Changed 18 months ago by yawning

https://bugzilla.mozilla.org/show_bug.cgi?id=336193

I don't have a good self contained way of making firefox exit gracefully. Maybe if I wait for another 10 fucking years, firefox will do a graceful shutdown on either SIGTERM or SIGINT.

My branch will display a modal dialog box asking the user to restart, because without modifying torbutton that's the best I can do.

comment:8 Changed 18 months ago by yawning

Well, using kill -9 seems to be ok, so the branch now prompts the user with a modal dialog box for a restart.

~It currently hits up the update metadata server too much during the actual update process, but suppressing the extra queries shouldn't be that hard.~

Edit: Ok, it hits up the update metadata server less now.

Last edited 18 months ago by yawning (previous) (diff)

comment:9 in reply to:  5 Changed 18 months ago by yawning

Replying to arma:

I think Tor Browser does its checks every 12 hours? It would be nice to match their schedule, since I think the metrics folks have been planning to estimate extant Tor Browser users by counting update checks and dividing by n.

The code leads me to believe that it is every 2h.
const kMinSecsBetweenChecks = 120 * 60; // 2.0 hours

The metrics thing is broken regardless because the way I check for updates (via the XML files that the update download process uses) is different from the way torbutton checks for updates (via the JSON file in the pref extensions.torbutton.versioncheck_url which currently points to https://www.torproject.org/projects/torbrowser/RecommendedTBBVersions).

I would rather not add extra code to check for updates a different way, when the current method works, and is required for the update process to work.

comment:10 Changed 18 months ago by yawning

Ok, so my plan as far as Tor Browser UI integration of the sandboxed update goes is:

  1. Introduce a new env var TOR_SANDBOX that my implementation will set to linux-v0.
  2. Modify the sandboxed-tor-browser control port surrogate to add:
    • _TBB_CHECKUPDATE, a command which will trigger (or display if already in progress) the sandboxed-tor-browser update check.
    • _TBB_UPDATE, an event that will fire if there is a browser update pending.
  3. Modify torbutton to (all actions assuming the new env var is set appropriately):
    • Send the _TBB_CHECKUPDATE command from torbutton_check_for_update() instead of calling into the prompter.
    • Register for the _TBB_UPDATE event, and set extensions.torbutton.updateNeeded to true if it fires.

I believe this mostly preserves the existing UX. I'm not sure what the right thing to do when the browser doesn't support integration would be, though I doubt my current strategy of "pop up a modal dialog box every 5 mins asking if the user wants to udpate" is the correct behavior.

nb: Since the new command and event are entirely synthetic and never hit the actual tor daemon, I'm not going to bother with a control-spec.txt patch (and since the surrogate doesn't allow events/names as a GETINFO query, the right way to check if this behavior should happen is "is the env var set to what I said I will set it to".

comment:11 Changed 18 months ago by yawning

Under this model update checks will:

  • Still be done solely by querying the XML thingies, instead of versioncheck_url (I *could* fetch that and discard the results on a timer, but that seems wasteful and stupid).
  • Occur:
    • At launch if the browser is known to be out of date (ie: The user exited instead of restarting to apply the update, after a check indicated that updates are available).
    • At launch if it has been > 2 hours past the last check.
    • Every 2 hours past the launch in the background.
    • If requested by the user via the torbutton menu item.

This is fairly close (barring the check URL being different), to existing behavior, except:

  • I use a timer, torbutton triggers off "new tab" and "new window".
  • I'll always wait 2 hours before hitting up the update server after launching firefox, torbutton can end up checking earlier (I could do the time math, if it's utterly important).

Assuming no one convinces me otherwise my plan moving forward is roughly:

  1. Do the sandboxed-tor-browser non-UI modifications required to support basic integration.
  2. Write the torbutton changes.
  3. (Maybe) Add support for downloading the MAR in the background (I might punt on this till a future version due to time/funding constraints).
  4. Do the sandboxed-tor-browser UI modifications, basically adding a dialog box to the background update checker.
  5. Change the behavior for old bundles that lack update integration to something other than "annoy the user with a modal dialog box every 5 mins".

comment:12 Changed 18 months ago by yawning

The contingency plan here would be something along the lines of:

  • Figure out a good way to signify to the user that an update is ready, and prompt them for a restart that super annoying/obnoxious.
  • Set the env var when launching sandboxed-tor-browser.
  • Write a tiny/trivial torbutton patch that just hides the Check for Tor Browser Update... menu entry if the env var is set.

And, at a later date, when time/funding is available, revisit my grand integration plans, and bump the env var value to linux-v1.

Regardless suggestions on what to do when Tor Browser -> sandbox integration does not exist would be appreciated.

comment:13 Changed 18 months ago by yawning

Instead of displaying an ugly dialog box, I wrote code to opportunistically use libnotify to inform the user that an update is available. This integrates better into most desktop environments, and is what other applications do.

libnotify was chosen because I'd rather deal with cgo than refactor the launcher around glib's GApplication (or the Gtk counterpart). The cost is another build time dependency, and a runtime optional dependency (the binding I wrote uses dlopen()/dlsym()), but the library is on my Debian/Fedora VMs so I suspect it's fairly ubiquitous, and the code will fall back to the ugly dialog box if required.

TODO: Notifications can have actions, so add an action to restart.

comment:14 in reply to:  7 ; Changed 18 months ago by yawning

Replying to yawning:

https://bugzilla.mozilla.org/show_bug.cgi?id=336193

I don't have a good self contained way of making firefox exit gracefully. Maybe if I wait for another 10 fucking years, firefox will do a graceful shutdown on either SIGTERM or SIGINT.

Ugh. I just managed to trigger this, unrelated to updates. I think I need to (in general) ask Tor Browser nicely to shoot itself in the head when tearing down the sandbox, so instead of the _TBB_UPDATE event, the code shall:

  • Register for the _TBB_SANDBOX event, and depending on the yet to be specified arguments, either set the pref or gracefully exit.

comment:15 in reply to:  14 Changed 18 months ago by yawning

I assume "Safe Mode" in the context of Tor Browser is basically totally broken (along with "Reset Firefox"), I just disabled the automatic prompt by setting a pref so that hopefully, it doesn't get into a totally messed up state when it gets killed.

https://gitweb.torproject.org/tor-browser/sandboxed-tor-browser.git/commit/?id=5dfacef0c56d958f1a5ee59d4ed2eb018160d738

This will hopefully make it ok to kill the firefox process with extreme prejudice and will stop the rare safemode prompts I see when testing.

comment:16 Changed 18 months ago by yawning

I spent some time trying to move from libnotify to glib's GApplication/GNotification, but it's a no go. The fundamental problem is g_notification_set_icon() doesn't appear to support using GBytesIcon, at least in a way that is compatible with the notification daemon I'm primarily testing with (the GNOME Flashback notification-daemon).

libnotify works fine, and is near ubiquitous, so this is more a minor rant for posterity than an actual problem.

comment:17 Changed 18 months ago by yawning

Description: modified (diff)
Summary: Check and download updates in the background.Check updates in the background.

I'm going to go with the contingency plan here, so I can work on other also useful things.. I'll file separate tickets for the background update download and full UI integration.

comment:18 Changed 18 months ago by yawning

Filed #21089 and #21090 for the follow up issues.

comment:19 Changed 18 months ago by yawning

Keywords: sandbox-update added
Note: See TracTickets for help on using tickets.