Skip to content
Snippets Groups Projects

add-on compatibility check occurs repeatedly

    • Closed (moved) Issue created by Mark Smith @mcs

    This is a spinoff of ticket #16014 (moved). Georg noticed that after he updated Tor Browser 4.5a5 to 5.0a1, he saw a "Checking Compatibility of Add-ons" window each time he started the browser. Kathy and I debugged this and found that this window is coming from the meek helper browser. It shows up repeatedly because the prefs.js file is not being written to the profile (presumably because the meek browser is killed and does not exit in a clean manner).

    One way to fix this is to add code to the meek HTTP helper extension that flushes the browser prefs. to disk before it enters the blocking event loop. There may be a better solution, but this seems to solve the problem.

    To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information

    Child items 0

    • No child items are currently assigned. Use child items to break down this issue into smaller parts.

    Activity

    • Mark Smith added component::circumvention/meek owner::dcf priority::medium resolution::fixed status::closed type::defect labels

      added component::circumvention/meek owner::dcf priority::medium resolution::fixed status::closed type::defect labels

    • D
      David Fifield @dcf ·

      Replying to mcs:

      One way to fix this is to add code to the meek HTTP helper extension that flushes the browser prefs. to disk before it enters the blocking event loop. There may be a better solution, but this seems to solve the problem.

      Does 0001-Bug-16269-add-on-compatibility-check-occurs-repeated.patch still work if savePrefFile(null) comes before, not after, setBoolPref("network.proxy.socks_remote_dns", false)? We actually don't want to save network.proxy.socks_remote_dns=false in prefs.js; it's meant to be a change in memory only.

      The reason we want network.proxy.socks_remote_dns=true in prefs.js, even though we always override that setting immediately, is to prevent DNS leaks in case something goes wrong and someone manages to run the meek headless browser, thinking it's a Tor Browser. See:

    • Mark Smith
      Mark Smith @mcs ·
      Author

      Replying to dcf:

      Does 0001-Bug-16269-add-on-compatibility-check-occurs-repeated.patch still work if savePrefFile(null) comes before, not after, setBoolPref("network.proxy.socks_remote_dns", false)?

      It should work.

      We actually don't want to save network.proxy.socks_remote_dns=false in prefs.js; it's meant to be a change in memory only.

      Hmmm. I wish there was a way to do this in a less fragile way. Would it be acceptable to use a default preferences file inside the meek HTTP extension to set network.proxy.socks_remote_dns = false?

      (I tried and failed to get that to work just now, although it should be possible)

    • D
      David Fifield @dcf ·

      Replying to mcs:

      Replying to dcf:

      We actually don't want to save network.proxy.socks_remote_dns=false in prefs.js; it's meant to be a change in memory only.

      Hmmm. I wish there was a way to do this in a less fragile way. Would it be acceptable to use a default preferences file inside the meek HTTP extension...

      That's exactly how it works now. The helper browser profile has its own prefs that are separate from the normal Tor Browser prefs. (It might inherit Tor Browser settings for anything that's unset, I'm not sure, but it overrides everything that matters for this ticket.)

      But I want the default in the helper's prefs to be network.proxy.socks_remote_dns=true, because that's a safe setting if something goes wrong (like the extension fails to load) and the headless browser unexpectedly appears on screen. If that happens, we at least want the browser to be non-functional (which it is, thanks to a default blackhole proxy setting) and not leak DNS (which it does not, thanks to network.proxy.socks_remote_dns=true). The main idea is that only the extension should be able to disable the safe default and make local DNS requests, because the extension knows what it's doing.

      We used to have network.proxy.socks_remote_dns=false, an unsafe fallback, but changed it in #12674 (moved). (I meant to link to #12674 (moved) in comment:1 but messed it up.) The way it works now is intended to make it so that if something breaks, it breaks in a safe way. https://gitweb.torproject.org/builders/tor-browser-bundle.git/tree/Bundle-Data/PTConfigs/meek-http-helper-user.js?id=0119a3c15711a66c76496d6e8e55511782140ec1#n21

      In sum, we want to save the updated prefs after an upgrade (to solve this ticket), but we don't want to save network.proxy.socks_remote_dns=false.

    • Mark Smith
      Mark Smith @mcs ·
      Author

      I understood what you were trying to do; I was just trying to find a more robust way to accomplish it. I did not know about the use of a user.js file. I think that will actually cause the network.proxy.socks_remote_dns pref. to be reset to true each time the browser starts up.

      In any case, Kathy and I tested a revised patch that flushes the prefs. to disk before the code in components/main.js resets network.proxy.socks_remote_dns. It seems to work fine and solve this issue. I will attach a patch for review.

    • Mark Smith
      Mark Smith @mcs ·
      Author

      Trac:
      0001-Bug-16269-add-on-compatibility-check-occurs-repeated.patch

    • Mark Smith
      Mark Smith @mcs ·
      Author

      a revised fix

    • Mark Smith
      Mark Smith @mcs ·
      Author

      Trac:
      Status: new to needs_review

    • D
      David Fifield @dcf ·

      Replying to mcs:

      I understood what you were trying to do; I was just trying to find a more robust way to accomplish it. I did not know about the use of a user.js file. I think that will actually cause the network.proxy.socks_remote_dns pref. to be reset to true each time the browser starts up.

      Oh, I think you are right. The user.js file should take precedence even if we accidentally write socks_remote_dns=false to prefs.js. That's fine, it's just another layer in the failsafe.

      In any case, Kathy and I tested a revised patch that flushes the prefs. to disk before the code in components/main.js resets network.proxy.socks_remote_dns. It seems to work fine and solve this issue. I will attach a patch for review.

      Thanks! I applied it and tagged 0.19 for you.

      For the record, this is what was added to my profile.meek-http-helper/prefs.js after I applied the patch. Before that, it was empty except for comments.

      user_pref("app.update.enabled", false);
user_pref("browser.cache.frecency_experiment", 1);
user_pref("browser.dom.window.dump.enabled", true);
user_pref("datareporting.healthreport.nextDataSubmissionTime", "1433523627528");
user_pref("datareporting.policy.firstRunTime", "1433437227527");
user_pref("extensions.blocklist.pingCountVersion", 0);
user_pref("extensions.enabledAddons", "meek-http-helper%40bamsoftware.com:1.0");
user_pref("extensions.installCache", "[{\"name\":\"app-profile\",\"addons\":{\"meek-http-helper@bamsoftware.com\":{\"descriptor\":\"/home/david/tor-browser_en-US/Browser/TorBrowser/Data/Browser/profile.meek-http-helper/extensions/meek-http-helper@bamsoftware.com\",\"mtime\":1433437213000,\"rdfTime\":946713600000}}}]");
user_pref("extensions.lastAppVersion", "31.7.0");
user_pref("extensions.lastPlatformVersion", "31.7.0");
user_pref("extensions.shownSelectionUI", true);
user_pref("network.proxy.socks_port", 9);
user_pref("security.ssl.disable_session_identifiers", false);
user_pref("toolkit.startup.max_resumed_crashes", -1);

      Trac:
      Status: needs_review to closed
      Resolution: N/A to fixed

    • Trac closed

      closed

    • Mark Smith mentioned in issue #16865 (moved)

      mentioned in issue #16865 (moved)

    • Trac moved to tpo/anti-censorship/pluggable-transports/meek#16269 (closed)

      moved to tpo/anti-censorship/pluggable-transports/meek#16269 (closed)

    Please register or sign in to reply