457377 - System-wide SOCKS proxy settings are not respected

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Daniel Luz]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080925021213 Minefield/3.1b1pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080925021213 Minefield/3.1b1pre

I have a network location setting where there's a SOCKS proxy setting (and no other proxies). Upon activating this location, other applications tested (Safari, Adium) recognize it immediately, without a restart. Firefox seems to completely ignore it.

I don't think this is relevant, but for completeness, this proxy is just pointing to localhost, using ssh dynamic port forwarding.

Reproducible: Always

Steps to Reproduce:
1. On OS X's Network Preferences, set to configure proxies manually, and assign a SOCKS proxy.
2. Make sure Firefox is set to "Use system proxy settings".
3. Test if the proxy is in effect. (For example, go to www.whatismyip.com and check if you are actually connecting through the remote machine.)
Actual Results:  
Firefox keeps connecting directly (or attempting to).

Expected Results:  
Firefox should respect SOCKS proxies. (I'm assuming it at the moment only considers the other proxies)

Comment 1

[Zandr Milewski [:zandr]]

It was reported to me by a client that network connections from JavaScript *do* obey the system proxy settings, but the browser itself does not. I'll try to come up with a reproduction case on Monday.

Comment 2

[Ludovic Hirlimann [:Usul]]

(In reply to comment #1)
> It was reported to me by a client that network connections from JavaScript *do*
> obey the system proxy settings, but the browser itself does not. I'll try to
> come up with a reproduction case on Monday.

Alexander are you still trying ?

Comment 3

[Zandr Milewski [:zandr]]

That ended up being a very confusing situation. I'll follow up and see what I can find out...the problems we saw might now be related.

Comment 4

[Andrew Shilliday]

I'm experiencing the same problem. 

Firefox:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090616 Firefox/3.5

System:
Darwin 9.7.0 Darwin Kernel Version 9.7.0: Tue Mar 31 22:52:17 PDT 2009; root:xnu-1228.12.14~1/RELEASE_I386 i386

To reproduce:
1) SSH to some machine using the -D option, e.g.,
    ssh -D <xxx> <user>@<host>
2) In System Preferences select Network -> Advanced -> Proxies
3) Under "Select a protocol to configure:" uncheck all but "SOCKS Proxy"
4) With "Socks Proxy" selected, set the SOCKS Proxy Server to localhost, and the port to <xxx>.
5) In Firefox select Preferences -> Network -> Settings
6) Select "Use System Proxy Settings"

Observed behavior: When browsing in Firefox, proxy settings are not used.
Expected behavior: That of Safari: proxy settings are used.

Comment 5

[Andrew Shilliday]

I'm no Firefox developer, and this comment comes from all of about 30 minutes looking at the source code, but it appears that the problem is in 
toolkit/system/osxproxy/nsOSXSystemProxySettings.mm.

There are two places where the code goes wrong (I think).  First is in line 205
if (NS_FAILED(aURI->SchemeIs(keys->mScheme, &res)) || !res) {

The problem here is that keys->mScheme, as it cycles through the list, is one of
"http", "https", "ftp", "socks", and null, but aURI->SchemIs will only ever return true for the first three.  My suggestion is to add a third clause to prevent the if block from being entered if keys->mScheme is "socks" which, I suppose, should be really appropriate for all uri schemes (correct?)

That's only the first problem. Later, around line 357, we see a call out to FindSCProxyPort which looks up the osx system settings and sets the proxyHost and proxyPort appropriately (the line above is part of that function).

357   rv = FindSCProxyPort(aURI, proxyHost, proxyPort);

359   if (NS_FAILED(rv) || IsInExceptionList(host)) {
360     aResult.AssignLiteral("DIRECT");
361   } else {
362     aResult.Assign(NS_LITERAL_CSTRING("PROXY ") + proxyHost + nsPrintfCString(":%d", proxyPort));
363   }

This *seems* to be constructing what would be the results of PAC file, i.e., statements of the form "DIRECT" or "PROXY <host>:<port>".  The issue is that when a SOCKS proxy is being used, the line should not begin with PROXY, but rather SOCKS (see http://docs.sun.com/app/docs/doc/820-1652/adyrr?a=view)

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Steven, is it an area you can help out or who is the right person?

Comment 7

[Steven Michaud [:smichaud] (Retired)]

I don't know much about this stuff ... but I might be able to help.

I don't know if we have a "right person".

Josh, what do you think?

Comment 8

[Andrew Shilliday]

Decided not to sleep and learn Objective C instead. Patch on its way.

Comment 9

[Andrew Shilliday]

Attachment: Patch for toolkit/system/osxproxy/nsOSXSystemProxySettings.mm

Address the issue that Firefox does not assign SOCKS system settings correctly.  There were three underlying problems. 
1) original code checked that url scheme matched either "http", "https", "ftp", or "socks", but no url will never match "socks", so the socks system settings were always ignored.  I added a clause so it would skip checking schemes against "socks".

2) When original code found a scheme match, it would check to make sure it was enabled in system preferences.  If it wasn't, it would break and return no proxy.  This means that if a url matched http, but http was disabled, the socks proxy would never be examined.  I changed code so that it continued the for loop to next proxy entry instead of breaking.  This way it checks each proxy entry for a match as well as whether it's enabled.

3) When proxy information is returned, orginal code had no way of knowing whether the proxy was SOCKS or a regular proxy, it always returned the string "PROXY <host>:<port>" but when a SOCKS proxy is used, it ought to return "SOCKS <host>:<port>" To fix this, I added an extra by-ref argument to the FindSCProxyPort function to return whether the host and port is a socks proxy.

Tested on my own machine and everything seems to work

Comment 10

[Andrew Shilliday]

As an aside, when Firefox is ready to change its system requirements to be OSX1.5 or later instead of OSX1.4, it will make sense to redo this code to use CFProxySupport

http://developer.apple.com/documentation/CoreFoundation/Reference/CFProxySupport/Reference/reference.html

The API allows you to just ask the system what the best proxy is given a URL, this would greatly simplify this code.

Comment 11

[Andrew Shilliday]

Comment on attachment 385096 [details] [diff] [review]
Patch for toolkit/system/osxproxy/nsOSXSystemProxySettings.mm

Added Benjamin Smedberg as reviewer

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Patch looks great. Cosmetic comments only:

@@ -48,18 +49,16 @@
 #include "nsISystemProxySettings.h"
 #include "nsIGenericFactory.h"
 #include "nsIServiceManager.h"
 #include "nsPrintfCString.h"
 #include "nsNetUtil.h"
 #include "nsISupportsPrimitives.h"
 #include "nsIURI.h"
 #include "nsObjCExceptions.h"
-
-
 class nsOSXSystemProxySettings : public nsISystemProxySettings {

Leave one blank line here

+    PRBool mIsSocksProxy;

Make it PRPackedBool and move it to the end

+    // Check whether we're using a SOCKS proxy

This comment is a bit misleading, it would be more like "return whether we're using a SOCKS proxy", probably just remove it.

Comment 13

[Andrew Shilliday]

Attachment: Next version of patch file, taking ROC's comments into account

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 385302 [details] [diff] [review]
Next version of patch file, taking ROC's comments into account

I think Josh wants to review this too.

Comment 15

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/2533ef04e498

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Something we could consider to have for 1.9.1.1?

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 385302 [details] [diff] [review]
Next version of patch file, taking ROC's comments into account

Approved for 1.9.1.4, a=dveditz for release-drivers

Comment 18

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4d743209b145

Comment 19

[Al Billings [:abillings - ex-MoCo]]

Daniel or Andrew, could you take the nightly 1.9.1 build and try it in your networked environment to see if this is fixed now?

ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-1.9.1/

Thanks.

Comment 20

[Andrew Shilliday]

Al, Done; All seems to work.

Comment 21

[Al Billings [:abillings - ex-MoCo]]

Thanks, Andrew. Marking as verified for 1.9.1.