Mac OS: wrong location for meek browser profile
With Tor Browser 6.0 and newer, Meek is not working at all for some Mac OS users. Anonymous said on https://blog.torproject.org/blog/tor-browser-60-released: Note to all OSX users encountering similar error: make sure before upgrade/clean install for 6.0+ that user installing has sudo privileges. TorBrowser needs write access to /Applications/TorBrowser-Data/ which will fail unless the user is an administrator. sudo privileges can be removed after installation and first run without problems. Kathy and I were able to reproduce this problem on Mac OS 10.11.5 using a clean install of Tor Browser 6.0 (placed in /Applications) when logged in as a non-admin user.
The problem is that the meek-client-torbrowser code that creates the Meek browser profile does not account for the fact that the user data should be stored under ~/Library/Application Support/TorBrowser-Data when the browser is in /Applications. In fact, Meek works correctly when running as a privileged (admin) user, but the Meek helper profile is incorrectly placed under /Applications/TorBrowser-Data/ (the main browser profile is correctly placed under ~/Library/Application Support/TorBrowser-Data).
Activity
A possible solution would be to modify Tor Launcher to set an environment variable that contains the path for the TorBrowser-Data directory, e.g., TOR_BROWSER_DATA_DIR='/Users/someone/Library/Application Support/TorBrowser-Data'
dcf: Would it be OK to make meek-client-torbrowser depend on such an env var? meek-client-torbrowser is only used by Tor Browser, correct?
Trac:
Status: new to needs_informationReplying to mcs:
A possible solution would be to modify Tor Launcher to set an environment variable that contains the path for the TorBrowser-Data directory, e.g., TOR_BROWSER_DATA_DIR='/Users/someone/Library/Application Support/TorBrowser-Data'
dcf: Would it be OK to make meek-client-torbrowser depend on such an env var? meek-client-torbrowser is only used by Tor Browser, correct?
Yes, this would be find.
meek-client-torbrowser is only used by Tor Browser. (Maybe it should move into a Tor Browser repo rather than a meek repo.)
Two patches are needed, one for meek (which I just attached to this ticket) and one for Tor Launcher, which is here: https://gitweb.torproject.org/user/brade/tor-launcher.git/commit/?h=bug19646-01&id=5ac452d86f93ee15ba3722afc606be862e8f3686
Please review both patches.
Trac:
Cc: dcf, brade, gk to dcf, brade, gk, arthuredelstein
Status: new to needs_review
Keywords: TorBrowserTeam201610 deleted, TorBrowserTeam201611R added
Applied the tor-launcher changes to master, maint-0.2.10 and maint-0.2.9 (commit 5ac452d86f93ee15ba3722afc606be862e8f3686, 4eee281bf774f8cf4f2d8a2549573b7a89f72407 and 0082643cd0b88f34cb56501b6631307616b8cc1d).
dcf: Could you prepare a new tag for meek with the commit up for review here (in case you think the changes are okay)? I am fine putting that into 6.0.6 as well. For this we need a new tag by next Tuesday, thanks. FWIW the changes look good to me, too, although I have a hard time parsing the second sentence of the commit message. Seems to be missing something there. But might be fixable just with a
git commit --amend.-
Replying to gk:
dcf: Could you prepare a new tag for meek with the commit up for review here (in case you think the changes are okay)? I am fine putting that into 6.0.6 as well. For this we need a new tag by next Tuesday, thanks. FWIW the changes look good to me, too, although I have a hard time parsing the second sentence of the commit message. Seems to be missing something there. But might be fixable just with a
git commit --amend.Oops. The commit message should read "This fixes a problem where meek-client-torbrowser...."
-
The meek patch, with an amended commit message as discussed in comment:11, is now available here: https://gitweb.torproject.org/user/brade/meek.git/commit/?h=bug19646-01&id=73e416fe9cb97d1bde9404b71b5dfe1957affc76 (availability in a git repo may make reviewing / testing / merging more convenient).
Replying to gk:
Applied the tor-launcher changes to master, maint-0.2.10 and maint-0.2.9 (commit 5ac452d86f93ee15ba3722afc606be862e8f3686, 4eee281bf774f8cf4f2d8a2549573b7a89f72407 and 0082643cd0b88f34cb56501b6631307616b8cc1d).
dcf: Could you prepare a new tag for meek with the commit up for review here (in case you think the changes are okay)? I am fine putting that into 6.0.6 as well. For this we need a new tag by next Tuesday, thanks. FWIW the changes look good to me, too, although I have a hard time parsing the second sentence of the commit message. Seems to be missing something there. But might be fixable just with a
git commit --amend.I pushed you tag 0.25.
Replying to brade:
Two patches are needed, one for meek (which I just attached to this ticket) and one for Tor Launcher, which is here: https://gitweb.torproject.org/user/brade/tor-launcher.git/commit/?h=bug19646-01&id=5ac452d86f93ee15ba3722afc606be862e8f3686
Please review both patches.
As a potential improvement, let me know what you think about this.
torDataDirFirefoxProfilePathis an empty string on linux and windows. It's only non-empty on mac. Therefore, the first branch of theifwill only ever be taken on mac:var profilePath string var torDataDir = os.Getenv("TOR_BROWSER_TOR_DATA_DIR") if torDataDir != "" && torDataDirFirefoxProfilePath != "" { profilePath = filepath.Join(torDataDir, torDataDirFirefoxProfilePath) } else { profilePath, err = filepath.Abs(firefoxProfilePath) if err != nil { return } }Does it make sense to remove the check for
torDataDirFirefoxProfilePath != "", and just makeTOR_BROWSER_TOR_DATA_DIRandtorDataDirFirefoxProfilePaththe primary way of choosing the path on all platforms?firefoxProfilePathcan remain as a fallback, only used ifTOR_BROWSER_TOR_DATA_DIRis not set. I say this because the tor-launcher patch unconditionally setsTOR_BROWSER_TOR_DATA_DIR, and it's surprising that the environment variable will be ignored on platforms other than mac.-
Replying to dcf:
Does it make sense to remove the check for
torDataDirFirefoxProfilePath != "", and just makeTOR_BROWSER_TOR_DATA_DIRandtorDataDirFirefoxProfilePaththe primary way of choosing the path on all platforms?firefoxProfilePathcan remain as a fallback, only used ifTOR_BROWSER_TOR_DATA_DIRis not set. I say this because the tor-launcher patch unconditionally setsTOR_BROWSER_TOR_DATA_DIR, and it's surprising that the environment variable will be ignored on platforms other than mac.This is a good question to raise. The reason Kathy and I wrote the code to only use
TOR_BROWSER_TOR_DATA_DIRon OSX is because on the other platforms the meek browser profile is not currently located under the Tor data directory. On OSX, we moved the location when we changed to theTorBrowser-Datastructure (#13252 (moved)) because we had to do migration anyway and because we thought it made more sense to put the profile under<tordatadir>/PluggableTransports/(it is more hidden there and less likely to be confused with the main Tor Browser profile).Someday when we move Linux and Windows to a similar scheme (see #18367 (moved) and #18369 (moved)) we should definitely modify
meek-client-torbrowserto only have one codepath. I created a new ticket for this (#20600 (moved)) as well as a parent ticket to track all of the related work (#20599 (moved)).