OSX updater: consider disabling privilege escalation
In Firefox 52 (since 49), the Firefox updater will attempt to gain elevated privileges on OSX if necessary to apply an update. See: https://bugzilla.mozilla.org/show_bug.cgi?id=394984
So far I have not tested this with an ESR52-based Tor Browser, but we should decide whether we want to leave this feature enabled or remove it before the first stable release of Tor Browser 7.0.
On Windows, we disabled similar code because (1) most Windows users probably do not install Tor Browser in a directory that requires admin privileges and (2) we did not want to audit the code (e.g., we did not want there to be a chance that someone could be tricked into granting more privileges, perhaps due to malware that took advantage of another security bug).
On OSX the situation is a little different because we do encourage people to drop TorBrowser.app into /Applications, which does require admin privileges. I personally use an account on OSX that has Admin privileges at all times, so updates work fine for me with TB 6.x and earlier... but that is not considered best security practice on OSX (actually, I usually do not install TB in /Applications at all because I keep several versions around to make it easier to triage bugs).
Cc: Tim and Linda who may also have some thoughts on this. To be sure, there is a security vs. usability tradeoff here.
Activity
Replying to mcs:
In Firefox 52 (since 49), the Firefox updater will attempt to gain elevated privileges on OSX if necessary to apply an update. If necessary. That's great. On Windows, we disabled similar code Not good. (You even disabled updater's helper for staged updates.) because (1) most Windows users probably do not install Tor Browser in a directory that requires admin privileges Most users do not install Tor Browser, because it is portable. But if they want to install it locally like on macOS, you've disabled that... and (2) we did not want to audit the code (e.g., we did not want there to be a chance that someone could be tricked into granting more privileges, perhaps due to malware that took advantage of another security bug). in updater or where? On OSX the situation is a little different because we do encourage people to drop TorBrowser.app into /Applications, which does require admin privileges. Installed instead Portable. It is bad. I personally use an account on OSX that has Admin privileges at all times, so updates work fine for me with TB 6.x and earlier... but that is not considered best security practice on OSX No comments. (actually, I usually do not install TB in /Applications at all because I keep several versions around to make it easier to triage bugs). Portable. That's good. (Hint: TB is Thunderbird.) Cc: Tim and Linda who may also have some thoughts on this. To be sure, there is a security vs. usability tradeoff here. Yawning's sandbox for security. Usability aspect here is that many users want to make TBB the default web browser, also to make it more secure to open web content from any app, and as a part of that, there is the need to install TBB as usual app, usually to the default location which usually needs admin privileges on any OS.
Replying to gk:
I wonder why nobody has reported problems about failing updates on macOS so far. I mean the issue is not new with the switch to ESR52.
Not many people use multiple user accounts, and even if they do, Tor Browser can only be used from one account on the same machine, due to SOCKSPort conflicts. And in previous TBB releases, everything was stored in the app bundle, so multiple users meant sharing your bookmarks and everything else as well.
And there are permissions issues:
When an admin installs Tor Browser by dropping it in the Applications folder, they become the owner, with the group "admin" and mask 750.
So a non-admin user can't use Tor Browser, because they can't even read the directory. See #21779 (moved).
But another admin user can use Tor Browser, but can't update it.
And if an admin changes the permissions on the Tor Browser folder, they probably know what to do when an update fails.
Thanks Tim. One more scenario which I just tested: if a non-admin user installs Tor Browser into /Applications they are prompted to authenticate as an administrator. After they do that, TorBrowser.app is owned by the non-admin user (which surprises me a little). But that does mean that the non-admin user can update.
Reading the first part of https://bugzilla.mozilla.org/show_bug.cgi?id=394984 again, the scenario mentioned there is that of Firefox being installed by an account that no longer exists. So maybe the need for privilege escalation is very limited, even if we fix #21779 (moved).
Replying to mcs:
Thanks Tim. One more scenario which I just tested: if a non-admin user installs Tor Browser into /Applications they are prompted to authenticate as an administrator. After they do that, TorBrowser.app is owned by the non-admin user (which surprises me a little). But that does mean that the non-admin user can update.
Reading the first part of https://bugzilla.mozilla.org/show_bug.cgi?id=394984 again, the scenario mentioned there is that of Firefox being installed by an account that no longer exists. So maybe the need for privilege escalation is very limited, even if we fix #21779 (moved).
So, do we think the risk of privilege escalation support is worth it? If not, how much work would it be to "back" this out?
-
Replying to gk:
So, do we think the risk of privilege escalation support is worth it? If not, how much work would it be to "back" this out?
Sorry for the delayed response. Kathy and I have been working on a patch to disable the OSX elevation code, similar to what we do on Windows (a complete back out of the Mozilla patches that added this feature would be quite painful). Changing the code as little as possible while still achieving the correct error reporting behavior turned out to be a little tricky, but I think we have a good patch now. We will post it here as soon as we finish one more round of testing.
Trac:
Status: new to assigned
Cc: brade, gk, teor, linda to brade, gk, teor, linda, arthuredelstein, boklm
Owner: tbb-team to mcs -
Here is the patch: https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug21940-01&id=9d7f583bd7aeea524c9ca0f8c83016b348e11e37 Some of the changes involve C++ code, so two reviews are needed. The patch is fairly small though.
Trac:
Status: assigned to needs_review
Keywords: TorBrowserTeam201705 deleted, TorBrowserTeam201705R added Usability aspect here is that many users want to make TBB the default web browser, also to make it more secure to open web content from any app, and as a part of that, there is the need to install TBB as usual app, usually to the default location which usually needs admin privileges on any OS.
I don't think that there is a usability issue here, but that it is a security issue that the browser team will make a decision on. A decision can be made without my consultation.
The average internet user (which is different from the average Tor user) will likely not know the difference between an application requiring admin privileges or not, and will not notice if it does request it. They just want to install the thing so that they can use it. Unless it requires a different installation pattern than everything they are used to, I don't think they will notice.
Generally, I think we should still do what is best in terms of security.
(I would like to clarify that "if we de-escalate then things will break and we need to fix them" != a usability issue, that's just technical work as a result of decisions made. Also "a decision that will have security implications" != a usability issue (even if people have preferences over it, and if you honor those preferences. I think that's just catering to the right userbase. A usability issue is when everything is technically working fine, and people still have a hard time using it--i.e. when tor launcher asks a user to choose bridges but they don't know what they are and choose randomly.)
-
Replying to mcs:
Here is the patch: https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug21940-01&id=9d7f583bd7aeea524c9ca0f8c83016b348e11e37 Some of the changes involve C++ code, so two reviews are needed. The patch is fairly small though.
Looks good to me. I did not test the patch (but assume you did :) ) but just compiled the code and looked over the changes.