Windows: staged update fails if Meek is enabled
On Windows, if Meek is enabled, the updater fails while trying to copy files to the staged "update applied" directory. Failure occurs because the file Browser/TorBrowser/Data/Browser/profile.meek-http-helper/parent.lock is in use. The updater will fallback to an unstaged update and users will be prompted to restart to apply the update. This works but a large updated/ directory is left behind which will not be deleted until another update is staged.
We can fix this problem by skipping this parent.lock file inside the updater when copying files.
Activity
Here is a fix (untested so far, but it should fix the problem): https://gitweb.torproject.org/user/brade/tor-browser.git/commit/?h=bug16014-01&id=0b5ce197900bcbec938ed4075c09dbeb03e965a0
This does not seem like an urgent fix, but we should pick it up as soon as possible so fewer updates will be affected in the future.
We may also want to tell Windows users that it is safe to delete a leftover /Browser/updated directory.
Trac:
Keywords: N/A deleted, TorBrowserTeam201505R added
Status: new to needs_review-
A brief update: we tested this proposed fix by installing TB 5.0a1 on a Windows 7 system, enabling meek-azure, adding an app.update.url.override that pointed to a test server that was configured to advertise a 5.0a2 update, and trying to update. After the staged update failed for the reason mentioned in this bug's description, we deleted the downloaded update info. from TorBrowser\Data\Browser\Caches\firefox, we replaced updater.exe with one that we built with the patch, and tried to update again. That time it succeeded without any trouble other than #13247 (moved).
Pending code review, Kathy and I think we should include this fix in 4.5.next and in the next 5.0 alpha.
Hrm... how can I reproduce the original problem? I took a Tor Browser 4.5a5, started it with meek-azure and performed an update to 5.0a1 on a Windows 7 machine. Worked like it should it seems (apart from the issue tracked in #13247 (moved)).
Trac:
Status: needs_review to needs_information-
Replying to gk:
Hrm... how can I reproduce the original problem?
Because the browser will fallback to an unstaged update (apply changes during restart), it may not be obvious that the staged update failed. You can set app.update.log=true and watch the Browser Console to see if a complaint is logged about a staged update failure. You can also check if a directory named updated/ is left behind after the update finishes.
-
Replying to gk:
{{{ 15:57 < GeKo> mcs: using my updater.exe I always get "status: failed: 19" and the update breaks. I wonder what happens and whether I messed up the .exe 15:57 < GeKo> mcs: do you have, by chance, your updater.exe with a hash sum somewhere to rule build failures on my side out? }}}
I am sorry I missed you on IRC. I no longer have the installed copy of TB on Windows that we used when testing the fix for this ticket, but I put a copy of the updater.exe from my builder tree plus its checksum here: https://pearlcrescent.com/tor/16014/ I am 99% sure that is what we used to test.
If you get error 19 (CERT_VERIFY_ERROR) it is may be that the wrong signing certificate was embedded in updater.exe. I have run into that before; alpha, beta, and release builds use release_primary.der and release_secondary.der but nightly builds use nightly_aurora_level3*.der.
Or you do are using an unsigned MAR. Or something else that we do not understand is happening.
-
Yeah, I just made a nightly build forgetting about the different certs, good. While the updated/ directory is gone there is now the problem that every start produces the check-your-extensions-compatibility-dialog which is quite annoying. This does not happen without using your updater.exe and we should avoid that. Btw: Do you know where this mysterious
tobedeleteddirectory is coming from with weird "temporary files" in it (happens without your updater.exe as well)? I just saw this inside /Browser after the update while testing. -
Replying to gk:
Yeah, I just made a nightly build forgetting about the different certs, good. While the updated/ directory is gone there is now the problem that every start produces the check-your-extensions-compatibility-dialog which is quite annoying. This does not happen without using your updater.exe and we should avoid that.
I do not see how the fix for this ticket could affect the extensions compatibility dialog. How can I reproduce that problem? Is it a dialog that stays open or is it the one that quickly opens and closes while checking for extension updates? (maybe I do not see it on my system because it closes too quickly)
Btw: Do you know where this mysterious
tobedeleteddirectory is coming from with weird "temporary files" in it (happens without your updater.exe as well)? I just saw this inside /Browser after the update while testing.Having a few files in "tobedeleted" is expected. If the updater is unable to delete a file on Windows because it is in use, it moves the file to a tobedeleted directory and arranges for the file to be deleted the next time Windows is restarted. The file is also renamed to so it has a temporary/unique name. This usually happens for updater.exe and the NSPR/NSS DLLs. While describing this to you, I just realized that this involves Windows registry changes. Should we create our own code to clean up the "tobedeleted" files?
-
Replying to mcs:
Replying to gk:
Yeah, I just made a nightly build forgetting about the different certs, good. While the updated/ directory is gone there is now the problem that every start produces the check-your-extensions-compatibility-dialog which is quite annoying. This does not happen without using your updater.exe and we should avoid that.
I do not see how the fix for this ticket could affect the extensions compatibility dialog. How can I reproduce that problem? Is it a dialog that stays open or is it the one that quickly opens and closes while checking for extension updates?
The latter. The weird thing is that I see it once on the Windows machine doing a "normal" update from 4.5a5 (with or without meek) which is quite normal. But using your updater.exe I see that dialog shortly with every start. If you think these things are unrelated, fine by me. I am just worried that the fix has some unintended side effects. :)
Btw: Do you know where this mysterious
tobedeleteddirectory is coming from with weird "temporary files" in it (happens without your updater.exe as well)? I just saw this inside /Browser after the update while testing.Having a few files in "tobedeleted" is expected. If the updater is unable to delete a file on Windows because it is in use, it moves the file to a tobedeleted directory and arranges for the file to be deleted the next time Windows is restarted. The file is also renamed to so it has a temporary/unique name. This usually happens for updater.exe and the NSPR/NSS DLLs. While describing this to you, I just realized that this involves Windows registry changes. Should we create our own code to clean up the "tobedeleted" files?
Dunno, I don't feel strongly here.
Trac:
Status: needs_information to needs_review -
I created #16236 (moved) to track the "tobedeleted" / Windows Registry issue.
-
Replying to gk:
The latter. The weird thing is that I see it once on the Windows machine doing a "normal" update from 4.5a5 (with or without meek) which is quite normal. But using your updater.exe I see that dialog shortly with every start. If you think these things are unrelated, fine by me. I am just worried that the fix has some unintended side effects. :)
I am also worried there is a side effect or another bug.
I think it would help a lot if Kathy and I could reproduce the problem. Can you provide a little more detail about how you set things up? I am thinking you did something like this:
- Downloaded TB 4.5a5 and replaced the updater.exe with the one from https://pearlcrescent.com/tor/16014/.
- Started the the browser and configured it to use meek-azure (but what you said earlier implies that meek is not required in order to experience the extension compatibility dialog bug)
- Checked for updates and restarted to apply when prompted.
- Restarted again to get meek to work correctly (due to #13247 (moved)).
- From now on you see the add-on compatibility check window each time you start Tor Browser.
Is that it? Or did you use a different server and mar file? Did you use an en-US package?
Also, I am not sure in general if it works to simply swap in a different updater.exe (I cannot remember if the updater has version information embedded in it or not). So maybe you did something else.
Here are a few things you can look at within your install where you see this problem:
- Look under \Browser\TorBrowser\Data\Browser\Caches\firefox\updates\0 and see if there is an update.status file (that directory should be empty after a successful update). If there is an update.status file, what is in it?
- Look at the log files up one level from there and see if there is anything interesting in them (or just make them available so Kathy and I can look).
- Check timestamps on the log files and maybe other files to ensure that an update is not occurring over and over again (that seems unlikely, because you would notice slowness).
- Set app.update.log to true and see if anything is logged during start up.
-
Replying to mcs:
Never mind. Kathy and I just reproduced the problem (basically by following the steps I mentioned in my previous comment). We are out of time for today but we will debug this soon.
This turned out to be a meek problem. See #16269 (moved). Georg, please let us know if are satisfied with the fix for this ticket.
-
I think the fix is fine. I am just wondering if we should land that one only if the one for #16269 (moved) lands as well given that there is somehow an interdependency. Or was it just by chance that using your modified updater.exe revealed that problem?
-
Replying to gk:
I think the fix is fine. I am just wondering if we should land that one only if the one for #16269 (moved) lands as well given that there is somehow an interdependency. Or was it just by chance that using your modified updater.exe revealed that problem?
I think they are separate problems but it is hard to be sure. There is apparently a codepath in the browser that causes the meek helper browser's prefs.js file to be written... but only sometimes. Maybe it happens more often with the fix for this ticket? I tried again and was not able to reproduce the problem (I have been working on #16269 (moved) using a installation that I made by copying the one where I reproduced the problem on May 29th).
In any case, I hope we can get both fixes in at the same time. I just posted a revised patch in #16269 (moved) for dcf to review.
mentioned in issue #16236 (moved)
mentioned in issue #16269 (moved)
mentioned in issue tpo/anti-censorship/pluggable-transports/meek#16269 (closed)
mentioned in issue tpo/applications/tor-browser#16236 (closed)
