SOCKS: add version selector to prefs

VERIFIED FIXED

Status

VERIFIED FIXED
17 years ago
14 years ago

People

(Reporter: smeredith, Assigned: smeredith)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

17 years ago
I've implemented SOCKS 4 (see
http://bugzilla.mozilla.org/show_bug.cgi?id=65583). The preferences dialog
currently allows for SOCKS server name. It also needs a way to select SOCKS 4 or
SOCKS 5. The value of the selection is network.proxy.socks_version, which should
be "4" or "5".

One way to implement the UI is to change the label of the edit control back to
"SOCKS Host" (from the newly changed "SOCKS v5 Host") and add a radio button for
the SOCKS version. Additionally, a suggestion was made to create a UI similar to
the AIM client, which has one edit control for the proxy server name, and radio
buttons for the protocol (something like: SOCKS 4, SOCKS 5, HTTP, and HTTPS.)
(Assignee)

Comment 1

17 years ago
I am taking ownership and adding nsenterprise keyword.
Assignee: sgehani → smeredith
Keywords: nsenterprise

Comment 2

17 years ago
I could have sworn this was filed somewhere already, but maybe that's just the
same issue for PAC, etc.
(Assignee)

Comment 3

17 years ago
Created attachment 42635 [details]
An image of the proposed UI change. Please comment.

Comment 4

17 years ago
Steve, I suggest that the first time a connection is made to a particular SOCKS
host, it should send a version identifier/method selection message assuming that
it is SOCKS v5 (VER = X'05'). If it works, store that information in the prefs.
If it doesn't work, try again with SOCKS v4. If that works, store *that*
information in the prefs. Then in any future session, if your currently set
version doesn't work, retry with the other version before failing.

That way, you don't have to bother/rely on the user knowing which particular
version a particular SOCKS host is, you don't have people's Mozilla
configuration breaking when somebody upgrades/downgrades their SOCKS proxy, and
you don't need these radio buttons at all. Which is just as well, since you
don't have room to display them in the prefs panel anyway -- already the bottom
of the Automatic proxy configuration URL field and button are being clipped in
the Mac Classic theme, even without the extra row for SOCKS version. (sairuh, is
there a bug on that already?)

If you can't do that, please comment in this bug and we'll go to plan B.
We can't. The socks protocol versions are not backwards compatible.

I don't think theres even an 'incorrect version' error result, although I may be
wrong.
(Assignee)

Comment 6

17 years ago
The SOCKS server will send a rejection reply, then drop your TCP connection.
That's a bit of a technical problem because of current implementation of the
SOCKS layer.

We considered trying this approach, but decided that there might be a compelling
reason for a user to select one or the other. However, I don't think we came up
with an example of such a reason.

Status: NEW → ASSIGNED

Comment 7

17 years ago
I do insecure wingate socks5/4 checking on my irc network.... the design for my
bot is very simple. On connect, the host is scanned for wingate (which i wont
discuss) and socks ports.

on port 1080, the binary packet "5 1 0" is sent to the host (assuming the
connection is open.) If the host replies "5 0" then it is known to be socks 5!
On replies "5 255" or "0 91 0 0 0 0 0 0" the host is closed and reopened for
smart fallback to socks 4 scans. Once open again, the following binary packet is
sent: "4 1 27 88 64 112 171 241" which will either reply "0 90 0 0 0 0 0 0" on
sucess or "0 91 0 0 0 0 0 0" on failure.

4 1 27 (hi port) 88 (lowport, 27*256+88=7000) 64.212.171.241
Maybe this will help out on the decision for the browser... let it decide incase
the user is unsure on what to pick. Just thought I would throw my 2 cents in!

Comment 8

17 years ago
There was a discussion about version scanning in another SOCKS bug. I think this
is a horrible idea because some SOCKS servers log errors on failed connection
attempts. Using a auto-handshake would potentially fill up a server with tons of
spurious requests.
(Assignee)

Comment 9

17 years ago
Another thing to think about: to fully support SOCKS 5, we need to be able to
specify a user name and password to authenticate to the SOCKS 5 server. We
currently only support unauthenticated connections. The hardest part about
adding username and password support is the UI. So far, this enhancement hasn't
been requested, but it is required by the SOCKS 5 spec.

Comment 10

17 years ago
NIM does this, so maybe we could poach some XUL from the commercial build...

Comment 11

17 years ago
What does 4.x do to determine version? In the dialog it just asks for socks host
in 4.x and not ther version.
A comment on the attached screenshot that I made on IRC the other day to Bradley 
Baetz:

Before checking in any more content to this preference panel please ensure that 
the panel content fits on Mac in BOTH skins. This is a non-optional requirement. 

(Assignee)

Comment 13

17 years ago
4.x only supports SOCKS 4.

What about the help text in the dialog? Is it usefull, or can we sacrafice it?

Comment 14

17 years ago
What "help" text?

Comment 15

17 years ago
Ok, it appears that plan A is unworkable if we don't want to spam SOCKS 4 
servers with error messages. Narf.

So, plan B. Plan B has three parts. Firstly, get rid of the useless group box. 
(Since it encloses everything, what exactly is it grouping the items *from*?) 
Secondly, get rid of the help text. (Giving something like this help text only 
misleads people into thinking that it's a good idea for them to try to use it 
when they haven't been told to -- and if they need to use it, they'll know what 
to put in each field anyway.) And thirdly, put the SOCKS row in alphabetical 
order above the SSL row.

That gives us a panel which looks like this:

Proxies :::::::::::::::::::::::::::::::::::::::::::

( ) _Direct connection to the Internet

(*) _Manual proxy configuration
       _FTP: [                      ] _Port: [    ]
    _Gopher: [                      ] Port:  [    ]
      _HTTP: [                      ] P_ort: [    ]
     SO_CKS: [                      ] Por_t: [    ]
             ( ) SOCKS v_4  (*) SOCKS v_5
       _SSL: [                      ] Po_rt: [    ]

    _No Proxy for: [                              ]
          Example: .mozilla.org, .net.nz

( ) _Automatic proxy configuration
       _URL: [                      ] ( _Reload )

:::::::::::::::::::::::::::::::::::::::::::::::::::

Note the alignment: all the protocol `:'s are aligned with the `:' in `URL:', 
and the SOCKS v4 radio button is aligned with the left of the SOCKS host field. 
While you're there, please change the no-proxy-for example as shown above: 
currently in Mac Classic it's clipped as `.yourcompany.com, .yourcompany.c...', 
which isn't very helpful.

BTW Bradley, when are you going to get around to fixing that `_P_o_r_t:' 
business? :-)

Comment 16

17 years ago
bbaetz had thoughts about the order we should use the prefs you enter (if you
have http and socks proxies, which is used first?). I argued for strict 4xp
compatability, b/c these are migrated prefs.

We need to solve that issue next, because it might be better to move SOCKS to
the bottom, and put a note indicating what order of precedence is used.
mpt: when you file that bug about rearranging this dialog entirely? :)

benc: One change at a time...

Guys, this is a simple patch. Lets not turn this bug into a "redo the proxy pref
pane" bug.
(Assignee)

Comment 18

17 years ago
Well, the new stuff has to fit, so there's going to be at least some rearranging.

Comment 19

17 years ago
changed summary for searchability.

bbaetz: by "one" change, I assume that you mean we should stick to discussing
the layout of the panel.

mtp: how about putting the order of elements as:

http, ssl, sockks? If SOCKS is used last, then it should be last...
Summary: Prefs page for proxies needs to allow SOCKS 4 or v5 selection. → SOCKS: add version selector to prefs

Comment 20

17 years ago
umm mpt, we need the SOCKS v4 and SOCKS v5 options for PAC as well, can that be
fixed? Or should we revisit it later when we actually support it? ;)
(Assignee)

Comment 21

17 years ago
In the spirit of PAC, it makes the most sense for the PAC file to specify the
version. Let's continue that discussion in bug 78176.
(Assignee)

Comment 22

17 years ago
Created attachment 43599 [details]
Proxies pref window with layout changes.
(Assignee)

Comment 23

17 years ago
So I've made the changes to the window layout as discussed, except that I left
the group box around the controls because all the other pref windows have group
boxes, and when I tried it without, it looks inconsistent. 

It looks good in modern and classic on Windows and Linux. It looks good on
classic Mac, but I haven't tried it on modern yet because mozilla crashes if I
press the Apply Theme button. :-(
(Assignee)

Comment 24

17 years ago
One last question: All other label say "Proxy" but SOCKS says "Host". Shouldn't
it also say "Proxy?"
(Assignee)

Updated

17 years ago
Depends on: 92358
You've swapped the access keys on the port of socks and ssl (_r_ vs _t_).

mpt was going to redesign this back in March, but I never saw the ASCII art :)
(Assignee)

Comment 26

17 years ago
No, I swapped the order of SOCKS and SSL per mpt. The access keys remain
attached to their original items.

Comment 27

17 years ago
When can we expect to see this in a build? The lack of Socks4 support means 
than no-one at a large, 3 letters-in-it's-name, company can use the program, 
since the firewall here is Socks v4.
kad: socks support is in the trunk build (see bug 65583). Theres just no UI for
the version pref - you have to set it manually.
(Assignee)

Comment 29

17 years ago
Well, there is a XUL bug 92358 blocking this. I could code a workaround into the
.js, but I'd rather not. Then I'll just need some reviews, and I'll check it in.

The actual SOCKS 4 support was checked in the other day: bug 65583. You can add
user_pref("network.proxy.socks_version", 4); 
to your prefs.js to use it.
Keywords: patch, review
(Assignee)

Comment 30

17 years ago
Created attachment 43678 [details] [diff] [review]
Proposed patch for previously attached screenshot.
QA Contact: sairuh → benc

Comment 31

17 years ago
My only comment on the patch is that the <spring/> tag has been deprecated, and
you should now use <spacer/> instead.  Other than that, r/sr=hewitt
(Assignee)

Comment 32

17 years ago
Created attachment 44890 [details] [diff] [review]
Changed <spring> to <spacer>. Sync with trunk.
socks is different from other proxies (techinically, its really a tunnel). It
should be last. And the port access keys should be in order, I guess.

r=bbaetz with the first change, and with or without the second.
(Assignee)

Comment 34

17 years ago
Checked in the fix.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 35

17 years ago
reassigning QA contact to myself
QA Contact: benc → trix

Comment 36

17 years ago
changing QA contact to lrg@netscape.com
QA Contact: trix → lrg

Comment 37

17 years ago
This preference is controllable and set properly by the UI currently.  Changes
in the pref value manually, are accurately displayed in the UI.  Verified on
MacOS, Win and Linux, marking as verified.

Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.