diff options
author | Yawning Angel <yawning@schwanenlied.me> | 2014-03-10 03:47:58 +0000 |
---|---|---|
committer | Gert Doering <gert@greenie.muc.de> | 2014-04-13 20:34:49 +0200 |
commit | 2903eba5dfe35c981329a833845e24de3882161a (patch) | |
tree | 10fa73b98e56c7e460087438ad38635d02f53af9 | |
parent | a95358af543b9106f4ef481e4556d1d03459d058 (diff) | |
download | openvpn-2903eba5dfe35c981329a833845e24de3882161a.tar.gz openvpn-2903eba5dfe35c981329a833845e24de3882161a.tar.xz openvpn-2903eba5dfe35c981329a833845e24de3882161a.zip |
Fix SOCKSv5 method selection
So, RFC 1928 doesn't say anything about the METHODS field in the Method
Selection message being ordered in terms of preference or anything, and
the server is free to pick any of the METHODS offered by the client.
Always sending a Method Selection message with NO AUTHENTICATION REQUIRED
and USERNAME/PASSWORD set is broken on two fronts:
* If the OpenVPN client can't handle the server picking USERNAME/PASSWORD
due to the credentials being missing, it shouldn't offer it to the
server.
* If the OpenVPN client has credentials, then it should always attempt to
authenticate. This is a security product. "You can misconfigure it and
it will work" is not acceptable. Setting a username/password when the
SOCKS server doesn't require/support that as an option is the user not
configuring it correctly, and should be treated as such.
Also verify that the SOCKS server returned the auth that was requested.
URL: https://github.com/OpenVPN/openvpn/pull/14
Fix trac #377, trac #148
Acked-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20140413130102.GR16637@greenie.muc.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/8488
Signed-off-by: Gert Doering <gert@greenie.muc.de>
-rw-r--r-- | src/openvpn/socks.c | 18 |
1 files changed, 15 insertions, 3 deletions
diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c index 1551da8..6e29e7a 100644 --- a/src/openvpn/socks.c +++ b/src/openvpn/socks.c @@ -189,10 +189,15 @@ socks_handshake (struct socks_proxy_info *p, char buf[2]; int len = 0; const int timeout_sec = 5; + ssize_t size; + + /* VER = 5, NMETHODS = 1, METHODS = [0 (no auth)] */ + char method_sel[3] = { 0x05, 0x01, 0x00 }; + if (p->authfile[0]) + method_sel[2] = 0x02; /* METHODS = [2 (plain login)] */ - /* VER = 5, NMETHODS = 2, METHODS = [0 (no auth), 2 (plain login)] */ - const ssize_t size = send (sd, "\x05\x02\x00\x02", 4, MSG_NOSIGNAL); - if (size != 4) + size = send (sd, method_sel, sizeof (method_sel), MSG_NOSIGNAL); + if (size != sizeof (method_sel)) { msg (D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port write failed on send()"); return false; @@ -252,6 +257,13 @@ socks_handshake (struct socks_proxy_info *p, return false; } + /* validate that the auth method returned is the one sent */ + if (buf[1] != method_sel[2]) + { + msg (D_LINK_ERRORS, "socks_handshake: Socks proxy returned unexpected auth"); + return false; + } + /* select the appropriate authentication method */ switch (buf[1]) { |