From 4ca89d78079432465c39aeb0e11f89ca1b0e988e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 10 Mar 2020 11:31:37 +0000 Subject: [PATCH 1/2] gsocks5proxy: Return G_IO_ERROR_PROXY_NEED_AUTH if anonymous auth fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a username and password are specified by the caller, `GSocks5Proxy` tells the server that it supports anonymous *and* username/password authentication, and the server can choose which it prefers. Otherwise, `GSocks5Proxy` only says that it supports anonymous authentication. If that’s not acceptable to the server, the code was previously returning `G_IO_ERROR_PROXY_AUTH_FAILED`. That error code doesn’t indicate to the caller that authentication might succeed were they to provide a username and password. Change the error handling to make that clearer. A fuller solution would be to expose more of the method negotiation in the `GSocks5Proxy` API, so that the caller can specify ahead of time which authentication methods they want to use. That can follow in issue #2059 though. Signed-off-by: Philip Withnall Fixes: #1988 --- gio/gsocks5proxy.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/gio/gsocks5proxy.c b/gio/gsocks5proxy.c index c58be8369..158254428 100644 --- a/gio/gsocks5proxy.c +++ b/gio/gsocks5proxy.c @@ -170,8 +170,22 @@ parse_nego_reply (const guint8 *data, *must_auth = TRUE; break; - case SOCKS5_AUTH_GSSAPI: case SOCKS5_AUTH_NO_ACCEPT: + if (!has_auth) + { + /* The server has said it accepts none of our authentication methods, + * but given the slightly odd implementation of set_nego_msg(), we + * actually only gave it the choice of %SOCKS5_AUTH_NONE, since the + * caller specified no username or password. + * Return %G_IO_ERROR_PROXY_NEED_AUTH so the caller knows that if + * they specify a username and password and try again, authentication + * might succeed (since we’ll send %SOCKS5_AUTH_USR_PASS next time). */ + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_PROXY_NEED_AUTH, + _("The SOCKSv5 proxy requires authentication.")); + return FALSE; + } + G_GNUC_FALLTHROUGH; + case SOCKS5_AUTH_GSSAPI: default: g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_PROXY_AUTH_FAILED, _("The SOCKSv5 proxy requires an authentication " From 400cd0a2e84716b4e1065530ba99570b813fb28f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 27 Feb 2020 12:16:41 +0000 Subject: [PATCH 2/2] gsocks5proxy: Fix SOCKS5 username/password authentication It was checking for the main SOCKS5 version number, rather than the subnegotiation version number. The username/password authentication protocol is described in https://tools.ietf.org/html/rfc1929. Spotted and diagnosed by lovetox. Signed-off-by: Philip Withnall Fixes: #1986 --- gio/gsocks5proxy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gio/gsocks5proxy.c b/gio/gsocks5proxy.c index 158254428..a6544df33 100644 --- a/gio/gsocks5proxy.c +++ b/gio/gsocks5proxy.c @@ -243,7 +243,7 @@ set_auth_msg (guint8 *msg, static gboolean check_auth_status (const guint8 *data, GError **error) { - if (data[0] != SOCKS5_VERSION + if (data[0] != SOCKS5_AUTH_VERSION || data[1] != SOCKS5_REP_SUCCEEDED) { g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_PROXY_AUTH_FAILED,