search

OpenVPN / openvpn

OpenVPN is an open source VPN daemon
http://openvpn.net
Other
10.91k stars 3k forks source link

Bug in Host: name for proxy directive #635

Open mal19992 opened 5 days ago

mal19992 commented 5 days ago

While working on issue #633 while testing via the stunnel I found a bug in proxy connect code.

With the settings

remote 1.1.1.1 1194
http-proxy proxy.server.name.from.config.com 8080 auto

I see in openvpn client log:

Oct 27 08:19:07 XXXXX openvpn[944495]: Send to HTTP proxy: 'CONNECT 1.1.1.1:1194 HTTP/1.0'
Oct 27 08:19:07 XXXXX openvpn[944495]: Send to HTTP proxy: 'Host: 1.1.1.1'

openvpn client uses the values of "remote" field both for CONNECT and for Host: value. This is incorrect. The Host: field is used on the proxy server for SNI for selecting apache virtual server used for http proxy. The value of the Host: field should be the name(or ip adderss) as proxy host name. I think the proper values of host is the the one from http-proxy value:

'CONNECT 1.1.1.1:1194 HTTP/1.0' 'Host: proxy.server.name.from.config.com'

ordex commented 4 days ago

I think this makes sense ("Host:" is a HTTP header and is related to the connection to the proxy, not to the VPN server), however, do you happen to have any reference about this in any RFC? That would greatly increase the confidence in this change.

mal19992 commented 4 days ago

Inspecting what firefox does -- it sends the same host in CONNECT and Host: "CONNECT forums.openvpn.net:443 HTTP/1.1" 200 Host: forums.openvpn.net:443

looking at RFC https://datatracker.ietf.org/doc/html/rfc7231#section-4.3.6 it is a little bit ambiguous, but their example also uses the same host name in CONNECT and Host:

This make very inconvenient to run a proxy on the same IP:post with web server, (this is a common practice for many small shops), but current RFC examples show redundant identical information in two completely different fields.

ordex commented 4 days ago

Well, I think this happens because the client makes a request as if it was making it to the final target (ins some way).

Anyway, I think this confirms the current behaviour is what is generally accepted as correct and therefore it wouldn't make sense to change it.