Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#12674 closed enhancement (fixed)

Neuter meek-http-helper's default proxy setting

Reported by: dcf Owned by: dcf
Priority: Low Milestone:
Component: Obfuscation/meek Version:
Severity: Keywords: easy TorBrowserTeam201408
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The headless meek-http-helper browser undoes Tor Browser's proxy setting:

https://gitweb.torproject.org/builders/tor-browser-bundle.git/blob/5400da654020a34edb9edee70a0583a89231c4fe:/Bundle-Data/PTConfigs/meek-http-helper-user.js#l7

// 0 is "No proxy".
user_pref("network.proxy.type", 0);

This setting used to be necessary in order for the HTTPS requests to be made on the network without themselves trying to go through the local tor proxy. However, since #12120, we set the proxy type individually for every request (including a "direct" non-proxy when TOR_PT_PROXY is unset), so it's no longer necessary to change the global setting.

A good reason to leave the proxy set is so if someone manages to start Firefox using the meek-http-helper profile as a normal non-headless browser, it should fail closed, and give "the proxy server is refusing connections" rather than acting as an unproxied browser.

Even better, we can set the proxy URL to 127.0.0.1:9, the discard port, so it will fail even closeder if tor happens to be running on the usual port set by Tor Browser.

Child Tickets

Attachments (1)

bug12674.patch (4.5 KB) - added by dcf 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 3 years ago by dcf

  • Keywords TorBrowserTeam201407 added
  • Status changed from new to needs_review

Please check and cherry-pick 279f775f41b9b9aa4ea42babd71058de35ba07f8. It blackholes the proxy settings in the meek-http-helper profile. It's really only a failsafe because normally the meek-http-helper extension prevents a browser window from appearing, even if the user activates the profile manually. However if something were to go wrong (the browser starting in safe mode for example), this commit will keep a user from accidentally using the browser.

Last edited 2 years ago by dcf (previous) (diff)

comment:2 Changed 3 years ago by dcf

Note to self: Maybe we can set network.proxy.socks_remote_dns=true in user.js, but set it to false only inside the extension itself.

comment:3 follow-up: Changed 3 years ago by gk

However it also means that DNS would leak, even with a blackhole socks
proxy, if the user typed in a URL. So we additionally blackhole the http
and ssl protocols, which will handle the most common mistakes. It
doesn't handle everything, for instance I tested an ftp URL and it made
a DNS request.

Hrm, so you avoid the DNS proxy bypass by adding the proxy for HTTP(S). Why not adding it for FTP as well if you are seeing DNS requests while you are at it?

comment:4 in reply to: ↑ 3 Changed 3 years ago by dcf

Replying to gk:

Hrm, so you avoid the DNS proxy bypass by adding the proxy for HTTP(S). Why not adding it for FTP as well if you are seeing DNS requests while you are at it?

I tried setting an ftp proxy, but the DNS still leaked. I don't know how an FTP proxy works, but maybe it doesn't use host names and so the client is obliged to resolve the name itself.

I think my idea in comment:2 might work better and would require blackholing the socks proxy only (but I haven't tried it yet). You can hold off on this ticket, if you like, while I try it.

Changed 3 years ago by dcf

comment:5 Changed 3 years ago by dcf

  • Keywords TorBrowserTeam201408 added; TorBrowserTeam201407 removed

Here's a patch that sets a blackhole proxy without any DNS leaks. It's two commits. The first just upgrades to 0.10, which has a patch to set network.proxy.socks_remote_dns=false within the meek-http-helper extension itself, so the pref can remain true in user.js. The second commit just sets a blackholed socks proxy as the only proxy, and sets network.proxy.socks_remote_dns=true in user.js.

Last edited 2 years ago by dcf (previous) (diff)

comment:6 Changed 3 years ago by gk

  • Resolution set to fixed
  • Status changed from needs_review to closed
Note: See TracTickets for help on using tickets.