488162 - DNS prefetch leaks information because it doesn't honour network.proxy.socks_remote_dns

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[jag (Peter Annema)]

With a SOCKS5 proxy and socks_remote_dns enabled (this has UI in SeaMonkey, use about:config in Firefox), all DNS look-ups should go through the SOCKS proxy. Some/most do, in fact, but the DNS prefetch look-ups don't.

The easiest fix is probably to just not do any prefetching if network.proxy.socks_host isn't empty and that pref is enabled. Is there any benefit to prefetch when using a SOCKS5 proxy in this manner? If so, bonus points if you can make the prefetch requests go through the proxy :-)

Easy way to test this is to use "ssh -D 8080 remotehost" which sets up a SOCKS5 proxy on localhost:8080 to remotehost, and use tcpdump to check on your traffic (scan for packets going to/from the local DNS server).

Comment 1

[jag (Peter Annema)]

Two scenarios:

1) You're using this to browse some local network with domain names that aren't externally visible, so the prefetches will fail, and anyone looking at the packets now has a little more information about the local network (and here you thought you were safe by using ssh -D).

2) You're at a coffee shop and using ssh -D so you don't have to trust the network's DNS server. Best case, sniffers can see where you're browsing to, worst case (haven't checked, hopefully not) the prefetched information preëmpts the remote look-up and now you're back to using the untrusted DNS.

Comment 2

[Jason Duell]

Not just SOCKS?  It sounds like we don't ever want to DNS prefetch when we're using any kind of HTTP proxy, unless I'm missing something here.

Comment 3

[Patrick McManus [:mcmanus]]

If you are concerned enough to be running ssh -D you meet the clue threshold to set network.dns.disablePrefetch

The bit about SOCKS is well taken, I will make sure it either goes through it or is implicitly disabled.

Comment 4

[jag (Peter Annema)]

Jason: hrm, currently there's a HTTP Proxy guard in nsHttpChannel::AsyncOpen(), but not in nsHTMLAnchorLink::BindToTree(), nor in nsContentSink::PrefetchDNS().

Patrick: network.dns.disablePrefetch doesn't seem to help with PrefetchHigh(). By design, I guess, though that makes the pref a bit of a misnomer, or at least not the right tool for this job. And no, it's not hard to find the pref once you realize not all DNS requests get proxied, but I wouldn't have noticed if I hadn't been checking tcpdump. I don't think "Firefox, the browser you have to double-check" makes for great marketing, so I'm looking forward to SOCKS support in DNS prefetch ;-)

Comment 5

[Eric Jung [:ericjung]]

(In reply to comment #3)
> The bit about SOCKS is well taken, I will make sure it either goes through it
> or is implicitly disabled.

Rather than always resolving through the SOCKS server, please consider adding an interface, similar to nsIProtocolProxyFilter, that would enable extensions and other clients to dynamically determine whether to resolve a given domain through a proxy or not. The interface's signature could be, for example:

nsIProxyInfo applyFilter(in nsIURI aURI, in nsIProxyInfo aProxy)

with a corresponding interface for registering/unregistering filters (like nsIProtocolProxyService.registerFilter() and nsIProtocolProxyService.unregisterFilter()).

Comment 6

[:Gavin Sharp [email: [e-mail]]]

That sounds like an entirely unrelated feature request. File a new bug about it?

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

This sounds like the sort of thing we should block on, yes.

Comment 8

[Eric Jung [:ericjung]]

(In reply to comment #6)
> That sounds like an entirely unrelated feature request. File a new bug about
> it?

https://bugzilla.mozilla.org/show_bug.cgi?id=489422

Comment 9

[Patrick McManus [:mcmanus]]

Attachment: Disable prefetches on proxy config and pref

Patch makes two changes:

* all prefetches are disabled when network.dns.disablePrefetch is set to true. Previously high priority requests were still allowed because they are not speculative in nature. (They are precursors to opening a connection that we have already decided to open). However, the optimization is not worth the risk of the non-intuitive behavior - disablePrefetch now always means disablePrefetch.

* Manual proxy configurations implicitly disable prefetching. In general the results of the prefetch are not available to the proxy anyhow. Dynamic configurations are a lot trickier - they require too much work to figure out whether or not they apply on a speculative basis (perhaps even requiring a dns lookup themselves). The existing tunables will have to serve this case.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 375829 [details] [diff] [review]
Disable prefetches on proxy config and pref

Fix spelling of "implicitly" in the checkin comment, and looks good.

Comment 11

[Patrick McManus [:mcmanus]]

Attachment: Disable prefetches on proxy config and pref [Checkin: Comment 12]

reflects review in comment 10

Comment 12

[Johnny Stenback (:jst)]

Committed to trunk and branch.

http://hg.mozilla.org/mozilla-central/rev/d984ce01b94b
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7aa4483585bd

Marking fixed.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

This checkin was in a range identified with a Ts Shutdown regression on Windows:

Regression: Ts Shutdown increase 27.64% on WINNT 5.1 Firefox3.5
   Previous results:
       357.263 from build 20090506145316 of revision 486b76052a94 at 2009-05-06 14:53:00
   New results:
       456.0 from build 20090506155401 of revision 7aa4483585bd at 2009-05-06 15:54:00
http://graphs-new.mozilla.org/graph.html#tests=[{"machine":32,"test":36,"branch":3},{"machine":33,"test":36,"branch":3},{"machine":34,"test":36,"branch":3},{"machine":35,"test":36,"branch":3},{"machine":48,"test":36,"branch":3}]&sel=1241564040,1241736840

Comment 14

[Eric Jung [:ericjung]]

Something that slipped through the cracks here:

If I implement nsIProtocolProxyFilter::applyFilter() and return:

Cc["@mozilla.org/network/protocol-proxy-service;1"]
  .getService(Ci.nsIProtocolProxyService).newProxyInfo("socks5", myhost,
myport,
  Ci.nsIProxyInfo.TRANSPARENT_PROXY_RESOLVES_HOST, 0, null)    

then dns prefetches aren't performed through the SOCKS proxy because  network.dns.disablePrefetch isn't set.

Bug 536093 has a minimal test case.

I can set this pref before returning such a result, but I don't know how that will scale for addons like FoxyProxy which can return all sorts of different values for each and every call to applyFilter() -- including null or "direct" which means use no proxy; i.e., reset the pref before returning.


536093