Opened 18 months ago

Closed 17 months ago

Last modified 8 months ago

#11242 closed defect (fixed)

If you upgrade TBB by overwriting an old one, it reads the old extensions.torbutton.updateNeeded

Reported by: anon Owned by: brade
Priority: major Milestone:
Component: TorBrowserButton Version:
Keywords: MikePerry201403R, tbb-helpdesk-frequent Cc: mcs, mikeperry
Actual Points: Parent ID:
Points:

Description

The check for a new Tor version opens a file on the file system for version information comparison.

There are scenarios where this file may be written to by an old version, or a exiting slowly version, as part of a TBB upgrade. (Or other scenarios where users do things totally unexpected and not recommended.)

Thus: when checking for newer versions, an old version is referenced, which will always claim to be out of date despite not being so.

---

Possible improvements:

If the version check including the version of the build of the checker/writer somehow this condition would be avoided. Note that keeping the version of the author/checker in the file itself could lead to race conditions requiring some thought, while a version in the file path would not conflict. (e.g. old and new write to version.txt compared to old write to git-abcdef12-version.txt and new write to git-f00ff00f-version.txt)

Child Tickets

Change History (13)

comment:1 Changed 18 months ago by anon

<arma> seems to me that writing the version-that-said-the-thing too will help things a lot. and is good engineering practice in general. see e.g. how tor handles its state file

comment:2 Changed 18 months ago by mcs

  • Cc mcs mikeperry added

I do not fully understand this bug report. Are you referring to the version check that is performed by Torbutton against the data retrieved from https://check.torproject.org/RecommendedTBBVersions? Or to another version check?

comment:3 Changed 18 months ago by arma

  • Summary changed from Fragile version checks lead to unexpected out of date advisement erroneously to Continued false positives in Tor Browser up-to-date check

He is referring to whatever process decides to make the exclamation mark flash over the green onion.

We're getting reports of people who upgrade getting told they need to upgrade. E.g.
https://blog.torproject.org/blog/tor-browser-353-released#comment-54860

Can you describe the mechanism for what Torbutton does when it decides it's out of date? That way we'll be in a better position to make recommendations on how to do it more robustly.

comment:4 follow-up: Changed 18 months ago by mikeperry

The version check downloads https://check.torproject.org/RecommendedTBBVersions and inspects the JSON list returned to see if the TBB version is present in that list. If our TBB version is absent, we're out of date. If it's present, we're up to date. If the file is unreachable or has errors, we also consider ourselves up to date.

I updated the version file prior to releasing 3.5.3 on either the blog or the website. Unless that update was slow to propagate to check.torproject.org from torbrowser.git, I do not know how this could fail. The only other thing I can think of is that users are downloading builds before we actually release them, from one of our home directories on people.torproject.org.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 18 months ago by arma

Replying to mikeperry:

The version check downloads https://check.torproject.org/RecommendedTBBVersions and inspects the JSON list returned to see if the TBB version is present in that list. If our TBB version is absent, we're out of date. If it's present, we're up to date. If the file is unreachable or has errors, we also consider ourselves up to date.

Yes, but then what? Does it write anything to disk? Or does it set some internal state in memory, and on the next launch it is stateless and will eventually fetch the tbbversions file again and notice again that it's out of date?

comment:6 in reply to: ↑ 5 ; follow-up: Changed 18 months ago by mcs

Replying to arma:

Yes, but then what? Does it write anything to disk? Or does it set some internal state in memory, and on the next launch it is stateless and will eventually fetch the tbbversions file again and notice again that it's out of date?

It toggles a local Boolean preference (extensions.torbutton.updateNeeded) based on the outcome of the remote check. That is done so Torbutton does not oscillate between reporting "out of date" and "up-to-date". To make things even worse, the problem is that the remote check is not repeated until 1.5 hours after the last check was done.

In the past, it is my understanding that "in place upgrades" were not really supported. But they work OK now, and even if someone just copies their prefs.js file over from old-TBB to new-TBB they will experience this bug.

The idea of marking the data with the version number of the thing that created it is a good one. A simple solution would be for Torbutton to store the current TBB version in a preference and invalidate (reset) other preferences such as extensions.torbutton.updateNeeded when it notices that the TBB version of the installed package has changed.

comment:7 in reply to: ↑ 6 Changed 18 months ago by arma

  • Summary changed from Continued false positives in Tor Browser up-to-date check to If you upgrade TBB by overwriting an old one, it reads the old extensions.torbutton.updateNeeded

Replying to mcs:

A simple solution would be for Torbutton to store the current TBB version in a preference and invalidate (reset) other preferences such as extensions.torbutton.updateNeeded when it notices that the TBB version of the installed package has changed.

Sounds great to me.

comment:8 Changed 18 months ago by brade

  • Keywords MikePerry201403R added
  • Status changed from new to needs_review

comment:9 Changed 18 months ago by mcs

  • Component changed from Tor Launcher to Torbutton

comment:10 Changed 18 months ago by mcs

  • Component changed from Torbutton to TorBrowserButton

comment:11 Changed 17 months ago by mttp

  • Keywords tbb-helpdesk-frequent added

comment:12 Changed 17 months ago by mikeperry

  • Resolution set to fixed
  • Status changed from needs_review to closed

Merged. Will appear in a nightly soon, and in the next TBB release.

Thanks!

comment:13 Changed 8 months ago by cypherpunks

This issue or one like it affects current releases: #12745

Note: See TracTickets for help on using tickets.