Proxy support patch

David Woodhouse dwmw2 at infradead.org
Fri Dec 18 04:19:56 EST 2009


On Fri, 2009-12-18 at 11:43 +1300, Pál Dorogi wrote:
> Hi,
> 
> I've created a patch for the openconnect-2.12 to allow connect to the
> target host through a Proxy.
> It's just a simple patch without any Proxy authentication feature.
> More info can be found in the
> http://ilapstech.blogspot.com/2009/12/add-proxy-support-to-openconnect.html

Looks good; thanks. A few comments...

It doesn't look like you've fixed the NetworkManager authentication
dialog to set the default proxy port, as main.c does. Perhaps it's
better to handle that in the connection code itself
(vpninfo->proxy_port?:"8080") rather than requiring the setup as you
have? But I'll settle for anything that doesn't make the NetworkManager
GUI segfault.

Don't you want to disable DTLS support when you connect through a proxy?
If direct TCP connections to port 443 don't work, then UDP surely isn't
going to either -- even if the server _would_ accept DTLS connections
from a different IP address to the CSTP connection, which I suspect it
doesn't.

You can't necessarily just use strchr(proxy,':') because that's going to
break parsing of IPv6 addresses. We need to make that work with a string
like '[2001:8b0:10b:1:21d:7dff:fe04:dbe2]:8080'

+ /* else the default one (1080) which is already set  will be used  */
Er, 8080? This isn't SOCKS support.

Shouldn't process_http_proxy() return zero for success, not 1?

This is going to immediately lead to feature requests / bug reports
about honouring an $http_proxy environment variable, and/or using
something like libproxy to "do the right thing" according to system-wide
configuration, including proxy autoconfig. Using libproxy ought to be
fairly simple, and it's small and simple enough that I think it's
reasonable to declare it an unconditional build dependency. Although
it's simple enough to have a fallback which just uses
getenv("http_proxy") if libproxy isn't available at build time.

It's going to lead to requests for SOCKS support too, but I can probably
do that later, if you want.

Oh, and ideally you'd send a patch made from the git repository, which
wouldn't include changes to the version.c file. But just excluding
version.c from the patch works too.

-- 
dwmw2




More information about the openconnect-devel mailing list