1432905 - Add pref to prevent localhost DNS lookup in nsProfileLock.cpp

Status: RESOLVED DUPLICATE of bug 1448783

(Toolkit :: Startup and Profile System, enhancement)

Description

[Arthur Edelstein [:arthur]]

For Tor Browser, we patch nsProfileLock.cpp to prevent bypass of the Tor proxy. Here's the patch:
https://torpat.ch/18800
And the original ticket:
https://trac.torproject.org/18800

We'd like to propose uplifting this patch, behind a pref.

Comment 1

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1432905-Stop-localhost-DNS-lookup-with-enable-pr.patch

Here's a patch for review, using the --enable-proxy-bypass-protection flag we recently added.

Comment 2

[Robert Strong (they/them - no direct email)]

Comment on attachment 8948851 [details] [diff] [review]
0001-Bug-1432905-Stop-localhost-DNS-lookup-with-enable-pr.patch

I'm not the best person to review this so I changed the review to someone that I think is.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8948851 [details] [diff] [review]
0001-Bug-1432905-Stop-localhost-DNS-lookup-with-enable-pr.patch

Review of attachment 8948851 [details] [diff] [review]:
-----------------------------------------------------------------

Why do you care about this DNS lookup? First and foremost, on most systems, I expect this actually doesn't lead to an actual DNS lookup. Second, this is entirely unrelated to browsing.

Comment 4

[Arthur Edelstein [:arthur]]

My understanding is this patch is a defense in depth, to make sure Tor Browser never bypasses the proxy. Adding mcs and brade in case they have any further comment on this.

Comment 5

[Mark Smith [:mcs]]

(In reply to Arthur Edelstein (Tor Browser dev) [:arthuredelstein] from comment #4)
> My understanding is this patch is a defense in depth, to make sure Tor
> Browser never bypasses the proxy. Adding mcs and brade in case they have any
> further comment on this.

"Defense in depth" is my understanding as well.

Comment 6

[Arthur Edelstein [:arthur]]

Mike, what's your view? Is this a patch you would accept for the reason given? Thanks.

Comment 7

[Mike Hommey [:glandium]]

I'm afraid "defense in depth" as an answer to comment 3 is incomplete and hand-wavy. It seems to me this is only a change for the sake of it.

Comment 8

[:Paolo Amadini]

I don't think we should add a preference for this, but there is value in always avoiding the native lookup to improve startup performance. Even if this bug is not an exact duplicate, we should consolidate these efforts in bug 1448783.